From acc38391dea12a7f70142077250d15a4eb53cb87 Mon Sep 17 00:00:00 2001
From: Carlos Goncalves <cgoncalves@redhat.com>
Date: Mon, 25 May 2020 20:47:34 +0200
Subject: [PATCH] Workaround peer name starting with hyphen

The base64_sha_string method is used to set a base64-encoded peer name
in HAProxy. There are cases where the peer name can start with
an hypen which is troublesome when used in HAProxy CLI. Specifically,
HAProxy fails to reload when local peer name starts with '-x' [1]. When
this is the case, an amphora goes to provisioning status ERROR and later
is scheduled for failover by the Octavia Health Manager service. A new
amphora UUUID is assigned and base64 encoded, hopefully not starting
with '-x' again. However, this is far from being ideal -- we incur in a
dataplane disruption (single topology) or reduce HA capabilities
(active-standby topology) for some time.

Four possible options:

a) add prefix to peer name
b) change b64encode altchars
c) quote peer name in haproxy CLI command
d) substitute first character if hyphen

Option a) and b) are not backward compatible with running amphorae. Peer
names of existing amphorae that do not start with hypen but contain
hyphen at any other position would get different peer names.

Option c) would nonetheless still require an amphora image update to add
quotes in the HAProxy init service file. Continuing to generate peer
names with hyphens at begininng of the string is avoidable and
recommended.

Option d), while also requiring an amphora image update, it would get
rid of hyphens in begining of the peer names. It is also backward
compatible with all running amphorae, except for those starting with
hyphen but are broken anyways.

This patch takes option d). It substitutes hyphen with 'x' character.

[1] https://github.com/haproxy/haproxy/issues/644

Task: 39850
Story: 2007714

Change-Id: Ib0fc26877710dea423a5ebcf1f71077665404377
---
 octavia/common/utils.py                           |  4 +++-
 octavia/tests/unit/common/test_utils.py           | 15 +++++++++++++++
 ...x-peer-name-prefix-hypen-e74a87e9a01b4f4c.yaml | 10 ++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/fix-peer-name-prefix-hypen-e74a87e9a01b4f4c.yaml

diff --git a/octavia/common/utils.py b/octavia/common/utils.py
index fde334d09c..112c64df72 100644
--- a/octavia/common/utils.py
+++ b/octavia/common/utils.py
@@ -45,7 +45,9 @@ def base64_sha1_string(string_to_hash):
     # break backwards compatibility with existing loadbalancers.
     hash_str = hashlib.sha1(string_to_hash.encode('utf-8')).digest()  # nosec
     b64_str = base64.b64encode(hash_str, str.encode('_-', 'ascii'))
-    return b64_str.decode('UTF-8')
+    b64_sha1 = b64_str.decode('UTF-8')
+    # https://github.com/haproxy/haproxy/issues/644
+    return re.sub(r"^-", "x", b64_sha1)
 
 
 def get_network_driver():
diff --git a/octavia/tests/unit/common/test_utils.py b/octavia/tests/unit/common/test_utils.py
index 512548314e..3d3add3985 100644
--- a/octavia/tests/unit/common/test_utils.py
+++ b/octavia/tests/unit/common/test_utils.py
@@ -89,3 +89,18 @@ class TestConfig(base.TestCase):
         exp_codes = '201-200, 205'
         self.assertEqual(utils.expand_expected_codes(exp_codes),
                          {'205'})
+
+    def test_base64_sha1_string(self):
+        str_to_sha1 = [
+            # no special cases str (no altchars)
+            ('77e7d60d-e137-4246-8a84-a25db33571cd',
+             'iVZVQ5AKmk2Ae0uGLP0Ue4OseRM='),
+            # backward compat amphorae with - in str[1:]
+            ('9c6e5f27-a0da-4ceb-afe5-5a81230be42e',
+             'NjrNgt3Egl-H5ScbYM5ChtUH3M8='),
+            # sha1 would start with -, now replaced with x
+            ('4db4a3cf-9fef-4057-b1fd-b2afbf7a8a0f',
+             'xxqntK8jJ_gE3QEmh-D1-XgCW_E=')
+        ]
+        for str, sha1 in str_to_sha1:
+            self.assertEqual(sha1, utils.base64_sha1_string(str))
diff --git a/releasenotes/notes/fix-peer-name-prefix-hypen-e74a87e9a01b4f4c.yaml b/releasenotes/notes/fix-peer-name-prefix-hypen-e74a87e9a01b4f4c.yaml
new file mode 100644
index 0000000000..0f883eab8f
--- /dev/null
+++ b/releasenotes/notes/fix-peer-name-prefix-hypen-e74a87e9a01b4f4c.yaml
@@ -0,0 +1,10 @@
+---
+upgrade:
+  - |
+    An amphora image update is recommended to pick up a workaround to an
+    HAProxy issue where it would fail to reload on configuration change should
+    the local peer name start with "-x".
+fixes:
+  - |
+    Workaround an HAProxy issue where it would fail to reload on configuration
+    change should the local peer name start with "-x".