From 0b7ca2624b8171056c92ac19c22ede2ab1e7ecc8 Mon Sep 17 00:00:00 2001
From: Felipe Reyes <felipe.reyes@canonical.com>
Date: Sat, 14 Dec 2019 21:49:49 -0300
Subject: [PATCH] Notify changes when service key is missing

Services that expose multiple endpoints use a prefix in their keys, this
patch refactors that code to put it in their own function to be reused
by the notifications functionality and make it notificate for changes in
those endpoints (e.g. neutron-api and nova-cloud-controller).

Change-Id: Ieecfc4ef7c85c7f716ceef0c2938ae0c7787953d
Closes-Bug: #1856419
---
 hooks/keystone_hooks.py           | 34 +++++++++------
 hooks/keystone_utils.py           | 72 +++++++++++++++++++++----------
 unit_tests/test_keystone_utils.py | 38 ++++++++++++++++
 3 files changed, 109 insertions(+), 35 deletions(-)

diff --git a/hooks/keystone_hooks.py b/hooks/keystone_hooks.py
index 9acb6cbf..1353b273 100755
--- a/hooks/keystone_hooks.py
+++ b/hooks/keystone_hooks.py
@@ -14,7 +14,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import hashlib
 import json
 import os
 import sys
@@ -126,6 +125,8 @@ from keystone_utils import (
     resume_unit_helper,
     remove_old_packages,
     stop_manager_instance,
+    assemble_endpoints,
+    endpoints_checksum,
 )
 
 from charmhelpers.contrib.hahelpers.cluster import (
@@ -440,18 +441,27 @@ def identity_changed(relation_id=None, remote_unit=None):
         if is_service_present('neutron', 'network'):
             delete_service_entry('quantum', 'network')
         settings = relation_get(rid=relation_id, unit=remote_unit)
-        service = settings.get('service', None)
+
+        # If endpoint has changed, notify to units related over the
+        # identity-notifications interface. We base the decision to notify on
+        # whether admin_url, public_url or internal_url have changed from
+        # previous notify.
+        service = settings.get('service')
         if service:
-            # If service is known and endpoint has changed, notify service if
-            # it is related with notifications interface.
-            csum = hashlib.sha256()
-            # We base the decision to notify on whether these parameters have
-            # changed (if csum is unchanged from previous notify, relation will
-            # not fire).
-            csum.update(settings.get('public_url', None).encode('utf-8'))
-            csum.update(settings.get('admin_url', None).encode('utf-8'))
-            csum.update(settings.get('internal_url', None).encode('utf-8'))
-            notifications['%s-endpoint-changed' % (service)] = csum.hexdigest()
+            key = '%s-endpoint-changed' % service
+            notifications[key] = endpoints_checksum(settings)
+        else:
+            # Some services don't set their name in the 'service' key in the
+            # relation, for those their name is calculated from the prefix of
+            # keys. See `assemble_endpoints()` for details.
+            single = {'service', 'region', 'public_url',
+                      'admin_url', 'internal_url'}
+            endpoints = assemble_endpoints(settings)
+            for ep in endpoints.keys():
+                if single.issubset(endpoints[ep]):
+                    key = '%s-endpoint-changed' % ep
+                    log('endpoint: %s' % ep)
+                    notifications[key] = endpoints_checksum(endpoints[ep])
     else:
         # Each unit needs to set the db information otherwise if the unit
         # with the info dies the settings die with it Bug# 1355848
diff --git a/hooks/keystone_utils.py b/hooks/keystone_utils.py
index dbf269ae..93d49567 100644
--- a/hooks/keystone_utils.py
+++ b/hooks/keystone_utils.py
@@ -14,6 +14,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import hashlib
 import json
 import os
 import shutil
@@ -1651,29 +1652,7 @@ def add_service_to_keystone(relation_id=None, remote_unit=None):
             https_cns.append(
                 urllib.parse.urlparse(settings['admin_url']).hostname)
     else:
-        # assemble multiple endpoints from relation data. service name
-        # should be prepended to setting name, ie:
-        #  realtion-set ec2_service=$foo ec2_region=$foo ec2_public_url=$foo
-        #  relation-set nova_service=$foo nova_region=$foo nova_public_url=$foo
-        # Results in a dict that looks like:
-        # { 'ec2': {
-        #       'service': $foo
-        #       'region': $foo
-        #       'public_url': $foo
-        #   }
-        #   'nova': {
-        #       'service': $foo
-        #       'region': $foo
-        #       'public_url': $foo
-        #   }
-        # }
-        endpoints = OrderedDict()  # for Python3 we need a consistent order
-        for k, v in settings.items():
-            ep = k.split('_')[0]
-            x = '_'.join(k.split('_')[1:])
-            if ep not in endpoints:
-                endpoints[ep] = {}
-            endpoints[ep][x] = v
+        endpoints = assemble_endpoints(settings)
 
         services = []
         for ep in endpoints:
@@ -2376,3 +2355,50 @@ def is_expected_scale():
     except NotImplementedError:
         return True
     return True
+
+
+def assemble_endpoints(settings):
+    """
+    Assemble multiple endpoints from relation data. service name
+    should be prepended to setting name, ie:
+     realtion-set ec2_service=$foo ec2_region=$foo ec2_public_url=$foo
+     relation-set nova_service=$foo nova_region=$foo nova_public_url=$foo
+
+    Results in a dict that looks like:
+    { 'ec2': {
+          'service': $foo
+          'region': $foo
+          'public_url': $foo
+      }
+      'nova': {
+          'service': $foo
+          'region': $foo
+          'public_url': $foo
+      }
+    }
+    """
+    endpoints = OrderedDict()  # for Python3 we need a consistent order
+    for k, v in settings.items():
+        ep = k.split('_')[0]
+        x = '_'.join(k.split('_')[1:])
+        if ep not in endpoints:
+            endpoints[ep] = {}
+        endpoints[ep][x] = v
+
+    return endpoints
+
+
+def endpoints_checksum(settings):
+    """
+    Calculate the checksum (sha256) of public_url, admin_url and internal_url
+    (in that order)
+
+    :param settings: dict with urls registered in keystone.
+    :returns: checksum
+    """
+    csum = hashlib.sha256()
+    log(str(settings))
+    csum.update(settings.get('public_url', None).encode('utf-8'))
+    csum.update(settings.get('admin_url', None).encode('utf-8'))
+    csum.update(settings.get('internal_url', None).encode('utf-8'))
+    return csum.hexdigest()
diff --git a/unit_tests/test_keystone_utils.py b/unit_tests/test_keystone_utils.py
index 33eba9a6..83792b2f 100644
--- a/unit_tests/test_keystone_utils.py
+++ b/unit_tests/test_keystone_utils.py
@@ -1531,3 +1531,41 @@ class TestKeystoneUtils(CharmTestCase):
             collections.OrderedDict([
                 ('file1', ['svc1']),
                 ('/etc/apache2/ssl/keystone/*', ['apache2'])]))
+
+    def test_assemble_endpoints(self):
+        data = {
+            "egress-subnets": "10.5.0.16/32",
+            "ingress-address": "10.5.0.16",
+            "neutron_admin_url": "http://10.5.0.16:9696",
+            "neutron_internal_url": "http://10.5.1.16:9696",
+            "neutron_public_url": "http://10.5.2.16:9696",
+            "neutron_region": "RegionOne",
+            "neutron_service": "neutron",
+            "private-address": "10.5.3.16",
+            "relation_trigger": "821bd014-3189-4062-a004-5b286335bcef",
+        }
+        expected = {
+            'neutron': {'service': 'neutron',
+                        'admin_url': "http://10.5.0.16:9696",
+                        'internal_url': "http://10.5.1.16:9696",
+                        'public_url': "http://10.5.2.16:9696",
+                        'region': 'RegionOne'},
+            # these are leftovers which are present in the response
+            'egress-subnets': {'': "10.5.0.16/32"},
+            'ingress-address': {'': "10.5.0.16"},
+            'relation': {'trigger': "821bd014-3189-4062-a004-5b286335bcef"},
+            'private-address': {'': "10.5.3.16"}
+        }
+        endpoints = utils.assemble_endpoints(data)
+        self.assertDictEqual(dict(endpoints), expected)
+
+    def test_endpoints_checksum(self):
+        settings = {'service': 'neutron',
+                    'admin_url': "http://10.5.0.16:9696",
+                    'internal_url': "http://10.5.1.16:9696",
+                    'public_url': "http://10.5.2.16:9696",
+                    'region': 'RegionOne'}
+        csum = utils.endpoints_checksum(settings)
+        self.assertEqual(csum,
+                         ('d938ff5656d3d9b50345e8061b4c73c8'
+                          '116a9c7fbc087765ce2e3a4a5df7cb17'))