From 712d88727c443502c1b55ec9aa075b9925e70df2 Mon Sep 17 00:00:00 2001 From: whoarethebritons Date: Mon, 7 Oct 2019 10:14:57 -0700 Subject: [PATCH 1/5] tasks should use http to avoid scheme redirects since they use HAProxy --- AppTaskQueue/appscale/taskqueue/push_worker.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/AppTaskQueue/appscale/taskqueue/push_worker.py b/AppTaskQueue/appscale/taskqueue/push_worker.py index 70fdeed0bd..cee0b5782a 100644 --- a/AppTaskQueue/appscale/taskqueue/push_worker.py +++ b/AppTaskQueue/appscale/taskqueue/push_worker.py @@ -152,16 +152,8 @@ def execute_task(task, headers, args): update_task(args['task_name'], TASK_STATES.FAILED) return - # Targets do not get X-Forwarded-Proto from nginx, they use haproxy port. - headers['X-Forwarded-Proto'] = url.scheme - if url.scheme == 'http': - connection = httplib.HTTPConnection(remote_host, url.port) - elif url.scheme == 'https': - connection = httplib.HTTPSConnection(remote_host, url.port) - else: - logger.error("Task %s tried to use url scheme %s, " - "which is not supported." % ( - args['task_name'], url.scheme)) + # Tasks should use HTTP to bypass scheme redirects since they use HAProxy. + connection = httplib.HTTPConnection(remote_host, url.port) skip_host = False if 'host' in headers or 'Host' in headers: From becceb43bb23bd61ad1597d16eb99f287801c929 Mon Sep 17 00:00:00 2001 From: Chris Donati Date: Mon, 14 Oct 2019 15:36:12 -0700 Subject: [PATCH 2/5] Use HAProxy location for the host header Since the host header defines which service the push task goes to in App Engine, this roughly preserves the same behavior (since the target service has already been selected at this point). There are some edge cases (especially during service relocation) where this behaves differently, but the routing pieces need more work to support those differences. --- AppTaskQueue/appscale/taskqueue/push_worker.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/AppTaskQueue/appscale/taskqueue/push_worker.py b/AppTaskQueue/appscale/taskqueue/push_worker.py index cee0b5782a..e38567c83e 100644 --- a/AppTaskQueue/appscale/taskqueue/push_worker.py +++ b/AppTaskQueue/appscale/taskqueue/push_worker.py @@ -155,17 +155,12 @@ def execute_task(task, headers, args): # Tasks should use HTTP to bypass scheme redirects since they use HAProxy. connection = httplib.HTTPConnection(remote_host, url.port) - skip_host = False - if 'host' in headers or 'Host' in headers: - skip_host = True - skip_accept_encoding = False if 'accept-encoding' in headers or 'Accept-Encoding' in headers: skip_accept_encoding = True connection.putrequest(method, urlpath, - skip_host=skip_host, skip_accept_encoding=skip_accept_encoding) # Update the task headers @@ -173,6 +168,14 @@ def execute_task(task, headers, args): headers['X-AppEngine-TaskExecutionCount'] = str(task.request.retries) for header in headers: + # Avoid changing the host header from the HAProxy location. Though GAE + # supports host-based routing, we need to make some additional changes + # before we can behave in a similar manner. Using the HAProxy location + # for the host header allows the dispatcher to try extracting a port, + # which it uses to set environment variables for the request. + if header == b'Host': + continue + connection.putheader(header, headers[header]) if 'content-type' not in headers or 'Content-Type' not in headers: From e0df16bf83a3461343603b9f5df5d6caeea75011 Mon Sep 17 00:00:00 2001 From: Chris Donati Date: Mon, 14 Oct 2019 15:39:15 -0700 Subject: [PATCH 3/5] Guess the intended scheme as a fallback This should only apply to the rare case when a client makes a request without going through Nginx and they've overridden the host header with a port-less value. --- AppServer/google/appengine/tools/devappserver2/module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AppServer/google/appengine/tools/devappserver2/module.py b/AppServer/google/appengine/tools/devappserver2/module.py index 77753bd57e..98ae1f5465 100644 --- a/AppServer/google/appengine/tools/devappserver2/module.py +++ b/AppServer/google/appengine/tools/devappserver2/module.py @@ -581,7 +581,7 @@ def _handle_request_impl(self, environ, start_response, inst=None, try: environ['SERVER_PORT'] = environ['HTTP_HOST'].split(':')[1] except IndexError: - scheme = environ['HTTP_X_FORWARDED_PROTO'] + scheme = environ.get('HTTP_X_FORWARDED_PROTO', 'http') if scheme == 'http': environ['SERVER_PORT'] = 80 else: From bdf54acb5d9da52d47ac4224dd26de06dd9ab7af Mon Sep 17 00:00:00 2001 From: Chris Donati Date: Tue, 15 Oct 2019 12:53:45 -0700 Subject: [PATCH 4/5] Update changelog and release notes --- RELEASE | 7 +++++++ VERSION | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/RELEASE b/RELEASE index 6623b5fd0f..0b2fd384f4 100644 --- a/RELEASE +++ b/RELEASE @@ -5,6 +5,13 @@ /_/ |_/ .___/ .___//____/\___/\__,_/_/ \___/ /_/ /_/ +AppScale version 3.8.1, released October 2019 +Highlights of features and defects fixed in this release: + - Fixes a regression in the 3.8 release where push tasks could not be executed + if the task URL had "secure: always" set. + +Known Issues: + AppScale version 3.8.0, released September 2019 Highlights of features and defects fixed in this release: - Login continue scheme diff --git a/VERSION b/VERSION index ae30a4da65..0c1ca211cc 100644 --- a/VERSION +++ b/VERSION @@ -4,4 +4,4 @@ / ___ |/ /_/ / /_/ /___/ / /__/ /_/ / // __/ /_/ |_/ .___/ .___//____/\___/\__,_/_/ \___/ /_/ /_/ -AppScale version 3.8.0 +AppScale version 3.8.1 From 29512a86f1ea6a1e5ad48a9ada50a085c179a169 Mon Sep 17 00:00:00 2001 From: Chris Donati Date: Wed, 16 Oct 2019 10:24:37 -0700 Subject: [PATCH 5/5] Restrict attrs version Hermes uses the "convert" attribute, which was removed in 19.2.0. --- Hermes/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Hermes/setup.py b/Hermes/setup.py index 22dfc96283..10e1272e94 100644 --- a/Hermes/setup.py +++ b/Hermes/setup.py @@ -15,7 +15,7 @@ 'kazoo', 'tornado', 'psutil==5.6.3', - 'attrs>=18.1.0', + 'attrs>=18.1.0,<19.2.0', # 19.2.0 removed "convert" attribute. 'mock', ], test_suite='appscale.hermes',