Merge lp:~cjwatson/launchpad/private-loggerhead into lp:launchpad

Proposed by Colin Watson
Status: Merged
Merged at revision: 18691
Proposed branch: lp:~cjwatson/launchpad/private-loggerhead
Merge into: lp:launchpad
Diff against target: 118 lines (+50/-8)
4 files modified
lib/launchpad_loggerhead/app.py (+19/-8)
lib/launchpad_loggerhead/tests.py (+25/-0)
lib/launchpad_loggerhead/wsgi.py (+1/-0)
lib/lp/services/config/schema-lazr.conf (+5/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad/private-loggerhead
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+345680@code.launchpad.net

Commit message

Make loggerhead listen on an additional private port without authentication.

Description of the change

The idea of this is to be able to use loggerhead as an API from Launchpad, mostly its JSON endpoints.

paste.httpserver couldn't listen on multiple ports without having to bring up entire new thread pools for each one, so I ported this to gunicorn. I've tried to keep the worker configuration as close to what was there before as possible, but of course we should keep an eye on poor old babaco's memory usage after deployment.

The one thing I couldn't figure out how to port was the NoLockingFileHandler business. In principle it would be possible to do this by going through all the existing loggers in LoggerheadLogger.setup and hacking out the acquire and release methods of their handlers. However, gunicorn's different thread management may avoid the problem, and the whole thing is extremely dubious anyway; bearing in mind that locks are used for more than just writing to log files, it wouldn't surprise me if it caused as many weird hangs as it prevented. My plan is to ignore this until we see a problem that can be ascribed to it.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) wrote :

Can you split out the gunicorn port into an earlier MP? We can get that landed and deployed now and separately work out the security around the private port.

review: Needs Fixing
Revision history for this message
Colin Watson (cjwatson) wrote :
Revision history for this message
Colin Watson (cjwatson) wrote :

loggerhead is now on gunicorn on production. I believe I understand the necessary port security changes now after my work on this last week, so I can take care of that if this is approved.

Revision history for this message
William Grant (wgrant) wrote :

We'll need to get production/qastaging/staging host firewalls sorted out before this can land.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/launchpad_loggerhead/app.py'
--- lib/launchpad_loggerhead/app.py 2017-01-14 15:16:36 +0000
+++ lib/launchpad_loggerhead/app.py 2018-06-06 12:47:56 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2017 Canonical Ltd. This software is licensed under the1# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4import logging4import logging
@@ -45,6 +45,7 @@
45from lp.code.interfaces.codehosting import (45from lp.code.interfaces.codehosting import (
46 BRANCH_TRANSPORT,46 BRANCH_TRANSPORT,
47 LAUNCHPAD_ANONYMOUS,47 LAUNCHPAD_ANONYMOUS,
48 LAUNCHPAD_SERVICES,
48 )49 )
49from lp.codehosting.safe_open import safe_open50from lp.codehosting.safe_open import safe_open
50from lp.codehosting.vfs import get_lp_server51from lp.codehosting.vfs import get_lp_server
@@ -185,6 +186,8 @@
185 raise HTTPMovedPermanently(next_url)186 raise HTTPMovedPermanently(next_url)
186187
187 def __call__(self, environ, start_response):188 def __call__(self, environ, start_response):
189 request_is_private = (
190 environ['SERVER_PORT'] == str(config.codebrowse.private_port))
188 environ['loggerhead.static.url'] = environ['SCRIPT_NAME']191 environ['loggerhead.static.url'] = environ['SCRIPT_NAME']
189 if environ['PATH_INFO'].startswith('/static/'):192 if environ['PATH_INFO'].startswith('/static/'):
190 path_info_pop(environ)193 path_info_pop(environ)
@@ -193,15 +196,23 @@
193 return favicon_app(environ, start_response)196 return favicon_app(environ, start_response)
194 elif environ['PATH_INFO'] == '/robots.txt':197 elif environ['PATH_INFO'] == '/robots.txt':
195 return robots_app(environ, start_response)198 return robots_app(environ, start_response)
196 elif environ['PATH_INFO'].startswith('/+login'):199 elif not request_is_private:
197 return self._complete_login(environ, start_response)200 if environ['PATH_INFO'].startswith('/+login'):
198 elif environ['PATH_INFO'].startswith('/+logout'):201 return self._complete_login(environ, start_response)
199 return self._logout(environ, start_response)202 elif environ['PATH_INFO'].startswith('/+logout'):
203 return self._logout(environ, start_response)
200 path = environ['PATH_INFO']204 path = environ['PATH_INFO']
201 trailingSlashCount = len(path) - len(path.rstrip('/'))205 trailingSlashCount = len(path) - len(path.rstrip('/'))
202 identity_url = environ[self.session_var].get(206 if request_is_private:
203 'identity_url', LAUNCHPAD_ANONYMOUS)207 # Requests on the private port are internal API requests from
204 user = environ[self.session_var].get('user', LAUNCHPAD_ANONYMOUS)208 # something that has already performed security checks. As
209 # such, they get read-only access to everything.
210 identity_url = LAUNCHPAD_SERVICES
211 user = LAUNCHPAD_SERVICES
212 else:
213 identity_url = environ[self.session_var].get(
214 'identity_url', LAUNCHPAD_ANONYMOUS)
215 user = environ[self.session_var].get('user', LAUNCHPAD_ANONYMOUS)
205 lp_server = get_lp_server(216 lp_server = get_lp_server(
206 identity_url, branch_transport=self.get_transport())217 identity_url, branch_transport=self.get_transport())
207 lp_server.start_server()218 lp_server.start_server()
208219
=== modified file 'lib/launchpad_loggerhead/tests.py'
--- lib/launchpad_loggerhead/tests.py 2018-06-06 08:57:53 +0000
+++ lib/launchpad_loggerhead/tests.py 2018-06-06 12:47:56 +0000
@@ -204,3 +204,28 @@
204 self.assertEqual(204 self.assertEqual(
205 "testopenid.dev:8085",205 "testopenid.dev:8085",
206 urlsplit(response.headers["Location"]).netloc)206 urlsplit(response.headers["Location"]).netloc)
207
208 def test_private_port_public_branch(self):
209 # Requests for public branches on the private port are allowed.
210 db_branch, _ = self.create_branch_and_tree()
211 branch_url = "http://127.0.0.1:%d/%s" % (
212 config.codebrowse.private_port, db_branch.unique_name)
213 response = requests.get(branch_url)
214 self.assertEqual(200, response.status_code)
215 title_tag = soupmatchers.Tag(
216 "page title", "title", text="%s : changes" % db_branch.unique_name)
217 self.assertThat(response.text, soupmatchers.HTMLContains(title_tag))
218
219 def test_private_port_private_branch(self):
220 # Requests for private branches on the private port are allowed.
221 db_branch, _ = self.create_branch_and_tree(
222 information_type=InformationType.USERDATA)
223 naked_branch = removeSecurityProxy(db_branch)
224 branch_url = "http://127.0.0.1:%d/%s" % (
225 config.codebrowse.private_port, naked_branch.unique_name)
226 response = requests.get(branch_url)
227 self.assertEqual(200, response.status_code)
228 title_tag = soupmatchers.Tag(
229 "page title", "title",
230 text="%s : changes" % naked_branch.unique_name)
231 self.assertThat(response.text, soupmatchers.HTMLContains(title_tag))
207232
=== modified file 'lib/launchpad_loggerhead/wsgi.py'
--- lib/launchpad_loggerhead/wsgi.py 2018-06-04 14:02:57 +0000
+++ lib/launchpad_loggerhead/wsgi.py 2018-06-06 12:47:56 +0000
@@ -105,6 +105,7 @@
105 "accesslog": os.path.join(log_folder, "access.log"),105 "accesslog": os.path.join(log_folder, "access.log"),
106 "bind": [106 "bind": [
107 "%s:%s" % (listen_host, config.codebrowse.port),107 "%s:%s" % (listen_host, config.codebrowse.port),
108 "%s:%s" % (listen_host, config.codebrowse.private_port),
108 ],109 ],
109 "errorlog": os.path.join(log_folder, "debug.log"),110 "errorlog": os.path.join(log_folder, "debug.log"),
110 # Trust that firewalls only permit sending requests to111 # Trust that firewalls only permit sending requests to
111112
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2018-05-31 13:14:12 +0000
+++ lib/lp/services/config/schema-lazr.conf 2018-06-06 12:47:56 +0000
@@ -199,6 +199,11 @@
199# datatype: int199# datatype: int
200port: 8080200port: 8080
201201
202# The port to listen on for private API requests.
203#
204# datatype: int
205private_port: 8090
206
202# A file path that contains a secret used for verifying cookies sent207# A file path that contains a secret used for verifying cookies sent
203# to codebrowse. This file should ideally contain 64 bytes of random208# to codebrowse. This file should ideally contain 64 bytes of random
204# data, and should be considered private -- if it is disclosed, we209# data, and should be considered private -- if it is disclosed, we