From 83af850ec2ef82040f7ae6fea39cc90361fd39fb Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Fri, 14 Jun 2019 13:37:29 -0700 Subject: [PATCH] Fix TCP listener logging bug HAProxy is not handling two of the HTTP log format variables correct when the load balancer has a TCP listener. This patch corrects that problem. Change-Id: I2eb8a0b5de46ee56321bc0009b6ca2b3ad4caebf --- octavia/common/constants.py | 3 +++ octavia/common/jinja/haproxy/jinja_cfg.py | 17 +++++++++++++++-- .../unit/common/jinja/haproxy/test_jinja_cfg.py | 12 ++++++------ 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/octavia/common/constants.py b/octavia/common/constants.py index 917ddcea86..84ea872238 100644 --- a/octavia/common/constants.py +++ b/octavia/common/constants.py @@ -658,3 +658,6 @@ SUPPORTED_CLIENT_AUTH_MODES = [CLIENT_AUTH_NONE, CLIENT_AUTH_OPTIONAL, CLIENT_AUTH_MANDATORY] TOPIC_AMPHORA_V2 = 'octavia_provisioning_v2' + +HAPROXY_HTTP_PROTOCOLS = [lib_consts.PROTOCOL_HTTP, + lib_consts.PROTOCOL_TERMINATED_HTTPS] diff --git a/octavia/common/jinja/haproxy/jinja_cfg.py b/octavia/common/jinja/haproxy/jinja_cfg.py index ebbd7ff186..8d544f8993 100644 --- a/octavia/common/jinja/haproxy/jinja_cfg.py +++ b/octavia/common/jinja/haproxy/jinja_cfg.py @@ -123,10 +123,22 @@ class JinjaTemplater(object): JINJA_ENV.filters['hash_amp_id'] = octavia_utils.base64_sha1_string return JINJA_ENV.get_template(os.path.basename(self.haproxy_template)) - def _format_log_string(self, load_balancer): + def _format_log_string(self, load_balancer, protocol): log_format = CONF.haproxy_amphora.user_log_format.replace( '{project_id}', load_balancer.project_id) log_format = log_format.replace('{lb_id}', load_balancer.id) + + # Order of these filters matter. + # TODO(johnsom) Remove when HAProxy handles the format string + # with HTTP variables in TCP listeners. + # Currently it either throws an error or just fails + # to log the message. + if protocol not in constants.HAPROXY_HTTP_PROTOCOLS: + log_format = log_format.replace('%{+Q}r', '-') + log_format = log_format.replace('%r', '-') + log_format = log_format.replace('%{+Q}ST', '-') + log_format = log_format.replace('%ST', '-') + log_format = log_format.replace(' ', '\ ') return log_format @@ -234,7 +246,8 @@ class JinjaTemplater(object): 'topology': listener.load_balancer.topology, 'amphorae': listener.load_balancer.amphorae, 'enabled': listener.enabled, - 'user_log_format': self._format_log_string(loadbalancer), + 'user_log_format': self._format_log_string(loadbalancer, + listener.protocol), 'timeout_client_data': ( listener.timeout_client_data or CONF.haproxy_amphora.timeout_client_data), diff --git a/octavia/tests/unit/common/jinja/haproxy/test_jinja_cfg.py b/octavia/tests/unit/common/jinja/haproxy/test_jinja_cfg.py index 102c5889bf..10edb4f142 100644 --- a/octavia/tests/unit/common/jinja/haproxy/test_jinja_cfg.py +++ b/octavia/tests/unit/common/jinja/haproxy/test_jinja_cfg.py @@ -298,7 +298,7 @@ class TestHaproxyCfg(base.TestCase): def test_render_template_https_real_monitor(self): fe = ("frontend sample_listener_id_1\n" " log-format 12345\\ sample_loadbalancer_id_1\\ %f\\ " - "%ci\\ %cp\\ %t\\ %{{+Q}}r\\ %ST\\ %B\\ %U\\ " + "%ci\\ %cp\\ %t\\ -\\ -\\ %B\\ %U\\ " "%[ssl_c_verify]\\ %{{+Q}}[ssl_c_s_dn]\\ %b\\ %s\\ %Tt\\ " "%tsc\n" " maxconn {maxconn}\n" @@ -334,7 +334,7 @@ class TestHaproxyCfg(base.TestCase): def test_render_template_https_hello_monitor(self): fe = ("frontend sample_listener_id_1\n" " log-format 12345\\ sample_loadbalancer_id_1\\ %f\\ " - "%ci\\ %cp\\ %t\\ %{{+Q}}r\\ %ST\\ %B\\ %U\\ " + "%ci\\ %cp\\ %t\\ -\\ -\\ %B\\ %U\\ " "%[ssl_c_verify]\\ %{{+Q}}[ssl_c_s_dn]\\ %b\\ %s\\ %Tt\\ " "%tsc\n" " maxconn {maxconn}\n" @@ -439,7 +439,7 @@ class TestHaproxyCfg(base.TestCase): def test_render_template_no_monitor_https(self): fe = ("frontend sample_listener_id_1\n" " log-format 12345\\ sample_loadbalancer_id_1\\ %f\\ " - "%ci\\ %cp\\ %t\\ %{{+Q}}r\\ %ST\\ %B\\ %U\\ " + "%ci\\ %cp\\ %t\\ -\\ -\\ %B\\ %U\\ " "%[ssl_c_verify]\\ %{{+Q}}[ssl_c_s_dn]\\ %b\\ %s\\ %Tt\\ " "%tsc\n" " maxconn {maxconn}\n" @@ -498,7 +498,7 @@ class TestHaproxyCfg(base.TestCase): def test_render_template_no_persistence_https(self): fe = ("frontend sample_listener_id_1\n" " log-format 12345\\ sample_loadbalancer_id_1\\ %f\\ " - "%ci\\ %cp\\ %t\\ %{{+Q}}r\\ %ST\\ %B\\ %U\\ " + "%ci\\ %cp\\ %t\\ -\\ -\\ %B\\ %U\\ " "%[ssl_c_verify]\\ %{{+Q}}[ssl_c_s_dn]\\ %b\\ %s\\ %Tt\\ " "%tsc\n" " maxconn {maxconn}\n" @@ -599,7 +599,7 @@ class TestHaproxyCfg(base.TestCase): def test_render_template_unlimited_connections(self): fe = ("frontend sample_listener_id_1\n" " log-format 12345\\ sample_loadbalancer_id_1\\ %f\\ " - "%ci\\ %cp\\ %t\\ %{{+Q}}r\\ %ST\\ %B\\ %U\\ " + "%ci\\ %cp\\ %t\\ -\\ -\\ %B\\ %U\\ " "%[ssl_c_verify]\\ %{{+Q}}[ssl_c_s_dn]\\ %b\\ %s\\ %Tt\\ " "%tsc\n" " maxconn {maxconn}\n" @@ -630,7 +630,7 @@ class TestHaproxyCfg(base.TestCase): def test_render_template_limited_connections(self): fe = ("frontend sample_listener_id_1\n" " log-format 12345\\ sample_loadbalancer_id_1\\ %f\\ " - "%ci\\ %cp\\ %t\\ %{+Q}r\\ %ST\\ %B\\ %U\\ " + "%ci\\ %cp\\ %t\\ -\\ -\\ %B\\ %U\\ " "%[ssl_c_verify]\\ %{+Q}[ssl_c_s_dn]\\ %b\\ %s\\ %Tt\\ " "%tsc\n" " maxconn 2014\n"