From d463897551017f6c4329e7109649af87e1ec88a9 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Sun, 28 Mar 2021 16:54:17 +0000 Subject: [PATCH] Fix pool ALPN compatibility with older amphora The ALPN for pools patch introduced a bug that causes amphora to fail if the HAProxy version running inside the amphroa does not support ALPN on backend members (bionic images for example). This patch adds compatibility support for those older HAProxy versions by removing the ALPN configuration settings if the HAProxy version is too old to support the functionality. Story: 2008780 Task: 42171 Change-Id: Iac949431f0901aaa0a60e5c39c2aab44cb9c1970 --- octavia/common/constants.py | 1 + .../haproxy/combined_listeners/jinja_cfg.py | 5 +- .../combined_listeners/test_jinja_cfg.py | 55 +++++++++++++++++-- ...l-alpn-older-haproxy-50514c1df4f77bcd.yaml | 8 +++ 4 files changed, 64 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/Fix-pool-alpn-older-haproxy-50514c1df4f77bcd.yaml diff --git a/octavia/common/constants.py b/octavia/common/constants.py index d153154c47..64a914e175 100644 --- a/octavia/common/constants.py +++ b/octavia/common/constants.py @@ -794,6 +794,7 @@ AMP_NETNS_SVC_PREFIX = 'amphora-netns' # Amphora Feature Compatibility HTTP_REUSE = 'has_http_reuse' +POOL_ALPN = 'has_pool_alpn' # TODO(johnsom) convert these to octavia_lib constants # once octavia is transitioned to use octavia_lib diff --git a/octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py b/octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py index 3005889574..6dfb72ba96 100644 --- a/octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py +++ b/octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py @@ -100,6 +100,8 @@ class JinjaTemplater(object): # Is it newer than haproxy 1.5? if not (int(haproxy_versions[0]) < 2 and int(haproxy_versions[1]) < 6): feature_compatibility[constants.HTTP_REUSE] = True + if not (int(haproxy_versions[0]) < 2 and int(haproxy_versions[1]) < 9): + feature_compatibility[constants.POOL_ALPN] = True return self.render_loadbalancer_obj( host_amphora, listeners, tls_certs=tls_certs, @@ -370,7 +372,8 @@ class JinjaTemplater(object): ret_value['tls_ciphers'] = pool.tls_ciphers if pool.tls_versions is not None: ret_value['tls_versions'] = pool.tls_versions - if pool.alpn_protocols is not None: + if (pool.alpn_protocols is not None and + feature_compatibility.get(constants.POOL_ALPN, False)): ret_value['alpn_protocols'] = ",".join(pool.alpn_protocols) if (pool.ca_tls_certificate_id and pool_tls_certs and pool_tls_certs.get('ca_cert')): diff --git a/octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py b/octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py index d68f960f6d..65ea92511c 100644 --- a/octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py +++ b/octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py @@ -1085,6 +1085,7 @@ class TestHaproxyCfg(base.TestCase): rendered_obj) def test_render_template_pool_cert(self): + feature_compatibility = {constants.POOL_ALPN: True} cert_file_path = os.path.join(self.jinja_cfg.base_crt_dir, 'sample_listener_id_1', 'fake path') be = ("backend sample_pool_id_1:sample_listener_id_1\n" @@ -1117,12 +1118,53 @@ class TestHaproxyCfg(base.TestCase): tls_certs={ 'sample_pool_id_1': {'client_cert': cert_file_path, - 'ca_cert': None, 'crl': None}}) + 'ca_cert': None, 'crl': None}}, + feature_compatibility=feature_compatibility) + self.assertEqual( + sample_configs_combined.sample_base_expected_config(backend=be), + rendered_obj) + + def test_render_template_pool_cert_no_alpn(self): + feature_compatibility = {constants.POOL_ALPN: False} + cert_file_path = os.path.join(self.jinja_cfg.base_crt_dir, + 'sample_listener_id_1', 'fake path') + be = ("backend sample_pool_id_1:sample_listener_id_1\n" + " mode http\n" + " balance roundrobin\n" + " cookie SRV insert indirect nocache\n" + " timeout check 31s\n" + " option httpchk GET /index.html HTTP/1.0\\r\\n\n" + " http-check expect rstatus 418\n" + " fullconn {maxconn}\n" + " option allbackups\n" + " timeout connect 5000\n" + " timeout server 50000\n" + " server sample_member_id_1 10.0.0.99:82 weight 13 " + "check inter 30s fall 3 rise 2 cookie sample_member_id_1 " + "{opts}\n" + " server sample_member_id_2 10.0.0.98:82 weight 13 " + "check inter 30s fall 3 rise 2 cookie sample_member_id_2 " + "{opts}\n\n").format( + maxconn=constants.HAPROXY_DEFAULT_MAXCONN, + opts="ssl crt %s verify none sni ssl_fc_sni" % cert_file_path + + " ciphers " + constants.CIPHERS_OWASP_SUITE_B + + " no-sslv3 no-tlsv10 no-tlsv11") + rendered_obj = self.jinja_cfg.render_loadbalancer_obj( + sample_configs_combined.sample_amphora_tuple(), + [sample_configs_combined.sample_listener_tuple( + pool_cert=True, tls_enabled=True, + backend_tls_ciphers=constants.CIPHERS_OWASP_SUITE_B)], + tls_certs={ + 'sample_pool_id_1': + {'client_cert': cert_file_path, + 'ca_cert': None, 'crl': None}}, + feature_compatibility=feature_compatibility) self.assertEqual( sample_configs_combined.sample_base_expected_config(backend=be), rendered_obj) def test_render_template_pool_cert_no_versions(self): + feature_compatibility = {constants.POOL_ALPN: True} cert_file_path = os.path.join(self.jinja_cfg.base_crt_dir, 'sample_listener_id_1', 'fake path') be = ("backend sample_pool_id_1:sample_listener_id_1\n" @@ -1155,12 +1197,14 @@ class TestHaproxyCfg(base.TestCase): tls_certs={ 'sample_pool_id_1': {'client_cert': cert_file_path, - 'ca_cert': None, 'crl': None}}) + 'ca_cert': None, 'crl': None}}, + feature_compatibility=feature_compatibility) self.assertEqual( sample_configs_combined.sample_base_expected_config(backend=be), rendered_obj) def test_render_template_pool_cert_no_ciphers(self): + feature_compatibility = {constants.POOL_ALPN: True} cert_file_path = os.path.join(self.jinja_cfg.base_crt_dir, 'sample_listener_id_1', 'fake path') be = ("backend sample_pool_id_1:sample_listener_id_1\n" @@ -1191,7 +1235,8 @@ class TestHaproxyCfg(base.TestCase): tls_certs={ 'sample_pool_id_1': {'client_cert': cert_file_path, - 'ca_cert': None, 'crl': None}}) + 'ca_cert': None, 'crl': None}}, + feature_compatibility=feature_compatibility) self.assertEqual( sample_configs_combined.sample_base_expected_config(backend=be), rendered_obj) @@ -1258,6 +1303,7 @@ class TestHaproxyCfg(base.TestCase): rendered_obj) def test_render_template_with_full_pool_cert(self): + feature_compatibility = {constants.POOL_ALPN: True} pool_client_cert = '/foo/cert.pem' pool_ca_cert = '/foo/ca.pem' pool_crl = '/foo/crl.pem' @@ -1294,7 +1340,8 @@ class TestHaproxyCfg(base.TestCase): 'sample_pool_id_1': {'client_cert': pool_client_cert, 'ca_cert': pool_ca_cert, - 'crl': pool_crl}}) + 'crl': pool_crl}}, + feature_compatibility=feature_compatibility) self.assertEqual( sample_configs_combined.sample_base_expected_config(backend=be), rendered_obj) diff --git a/releasenotes/notes/Fix-pool-alpn-older-haproxy-50514c1df4f77bcd.yaml b/releasenotes/notes/Fix-pool-alpn-older-haproxy-50514c1df4f77bcd.yaml new file mode 100644 index 0000000000..d7418ebfad --- /dev/null +++ b/releasenotes/notes/Fix-pool-alpn-older-haproxy-50514c1df4f77bcd.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixed amphora driver pool ALPN compatibity with older amphora images. +upgrade: + - | + Support for new features, such as ALPN on pools, HTTP/2 on pools, + gRPC, and SCTP require an updated amphora image.