From c4a25b1135524621e5c6e85d6424e7aa02c163c4 Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Wed, 10 May 2023 15:50:31 +0200 Subject: [PATCH 1/2] Update method for checking endpoint protocol (#769) (#775) Update method for checking endpoint protocol The `https` method is used to check if an endpoint is expected to be http or https. One of the checks it performs is to examine the the certificates relation. If the relation is present then it looks for the existance of a CA. However the OpenStack charms do not switch to https until a certificate is provided via the certificates relation. This means there can be a disconnect if the certificate provider has provided a CA but has not yet provided the unit specific certificates. If this happens then the payload will still be using http but the `https` method will return True. This patch updates the `https` method to return False if an unfilled certificate request exists. (cherry picked from commit 6064a34627882d1c8acf74644c48d05db67ee3b4) Co-authored-by: Liam Young (cherry picked from commit ed01437357921a95f2dedcff1ec50ae68ef46a53) --- charmhelpers/contrib/hahelpers/cluster.py | 7 +++++ tests/contrib/hahelpers/test_cluster_utils.py | 26 +++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/charmhelpers/contrib/hahelpers/cluster.py b/charmhelpers/contrib/hahelpers/cluster.py index a43be1689..a21b5b82f 100644 --- a/charmhelpers/contrib/hahelpers/cluster.py +++ b/charmhelpers/contrib/hahelpers/cluster.py @@ -224,6 +224,13 @@ def https(): return True if config_get('ssl_cert') and config_get('ssl_key'): return True + # Local import to avoid ciruclar dependency. + import charmhelpers.contrib.openstack.cert_utils as cert_utils + if ( + cert_utils.get_certificate_request() and not + cert_utils.get_requests_for_local_unit("certificates") + ): + return False for r_id in relation_ids('certificates'): for unit in relation_list(r_id): ca = relation_get('ca', rid=r_id, unit=unit) diff --git a/tests/contrib/hahelpers/test_cluster_utils.py b/tests/contrib/hahelpers/test_cluster_utils.py index 990f1dcb0..c033740f8 100644 --- a/tests/contrib/hahelpers/test_cluster_utils.py +++ b/tests/contrib/hahelpers/test_cluster_utils.py @@ -231,8 +231,11 @@ def test_https_cert_key_in_config(self): ] self.assertTrue(cluster_utils.https()) - def test_https_cert_key_in_identity_relation(self): + @patch('charmhelpers.contrib.openstack.cert_utils') + def test_https_cert_key_in_identity_relation(self, cert_utils): '''It determines https is available if cert in identity-service''' + cert_utils.get_certificate_request.return_value = False + cert_utils.get_requests_for_local_unit.return_value = {} self.config_get.return_value = False self.relation_ids.return_value = 'identity-service:0' self.relation_list.return_value = 'keystone/0' @@ -244,8 +247,27 @@ def test_https_cert_key_in_identity_relation(self): ] self.assertTrue(cluster_utils.https()) - def test_https_cert_key_incomplete_identity_relation(self): + @patch('charmhelpers.contrib.openstack.cert_utils') + def test_https_cert_req_pending(self, cert_utils): + '''It determines https is available if cert in identity-service''' + cert_utils.get_certificate_request.return_value = True + cert_utils.get_requests_for_local_unit.return_value = {} + self.config_get.return_value = False + self.relation_ids.return_value = 'identity-service:0' + self.relation_list.return_value = 'keystone/0' + self.relation_get.side_effect = [ + 'yes', # relation_get('https_keystone') + 'cert', # relation_get('ssl_cert') + 'key', # relation_get('ssl_key') + 'ca_cert', # relation_get('ca_cert') + ] + self.assertFalse(cluster_utils.https()) + + @patch('charmhelpers.contrib.openstack.cert_utils') + def test_https_cert_key_incomplete_identity_relation(self, cert_utils): '''It determines https unavailable if cert not in identity-service''' + cert_utils.get_certificate_request.return_value = False + cert_utils.get_requests_for_local_unit.return_value = {} self.config_get.return_value = False self.relation_ids.return_value = 'identity-service:0' self.relation_list.return_value = 'keystone/0' From cb3840a3c6a1c15c3bd1dced320b93de06808efa Mon Sep 17 00:00:00 2001 From: Edward Hope-Morley Date: Tue, 6 Jun 2023 15:51:53 +0100 Subject: [PATCH 2/2] Support legacy cert requests The tls-certificates interface supports two ways of requesting certs and openstack.cert_utils needs to support both when checking if certificates are ready to be used. Without this it will never declare certificates as ready if they were requested with the old/legacy method. Closes-Bug: #2022980 (cherry picked from commit 58097330bd15cfd7b539481c55718c6cee8ca124) --- charmhelpers/contrib/openstack/cert_utils.py | 11 ++++++ tests/contrib/openstack/test_cert_utils.py | 37 ++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/charmhelpers/contrib/openstack/cert_utils.py b/charmhelpers/contrib/openstack/cert_utils.py index 5c961c589..a25ca995e 100644 --- a/charmhelpers/contrib/openstack/cert_utils.py +++ b/charmhelpers/contrib/openstack/cert_utils.py @@ -409,6 +409,9 @@ def get_requests_for_local_unit(relation_name=None): relation_name = relation_name or 'certificates' bundles = [] for rid in relation_ids(relation_name): + sent = relation_get(rid=rid, unit=local_unit()) + legacy_keys = ['certificate_name', 'common_name'] + is_legacy_request = set(sent).intersection(legacy_keys) for unit in related_units(rid): data = relation_get(rid=rid, unit=unit) if data.get(raw_certs_key): @@ -416,6 +419,14 @@ def get_requests_for_local_unit(relation_name=None): 'ca': data['ca'], 'chain': data.get('chain'), 'certs': json.loads(data[raw_certs_key])}) + elif is_legacy_request: + bundles.append({ + 'ca': data['ca'], + 'chain': data.get('chain'), + 'certs': {sent['common_name']: + {'cert': data.get(local_name + '.server.cert'), + 'key': data.get(local_name + '.server.key')}}}) + return bundles diff --git a/tests/contrib/openstack/test_cert_utils.py b/tests/contrib/openstack/test_cert_utils.py index 7ed43c821..84bcc9507 100644 --- a/tests/contrib/openstack/test_cert_utils.py +++ b/tests/contrib/openstack/test_cert_utils.py @@ -579,6 +579,43 @@ def test_get_requests_for_local_unit(self, relation_get, relation_ids, 'chain': 'MYCHAIN'}] ) + @mock.patch.object(cert_utils, 'local_unit') + @mock.patch.object(cert_utils, 'related_units') + @mock.patch.object(cert_utils, 'relation_ids') + @mock.patch.object(cert_utils, 'relation_get') + def test_get_requests_for_local_unit_legacy(self, relation_get, + relation_ids, related_units, + local_unit): + local_unit.return_value = 'rabbitmq-server/2' + + def fake_relation_get(rid, unit): + if unit == 'rabbitmq-server/2': + # i.e. legacy request + return {'certificate_name': + 'eb32103b-27c8-4feb-8771-c7097c7314e8', + 'common_name': 'juju-cd4bb3-5.lxd', + 'sans': '["10.5.100.11", "10.5.0.32"]'} + else: + return { + 'rabbitmq-server_2.server.cert': 'BASECERT', + 'rabbitmq-server_2.server.key': 'BASEKEY', + 'chain': 'MYCHAIN', + 'ca': 'ROOTCA'} + + relation_ids.return_value = ['certificates:12'] + related_units.return_value = ['vault/0'] + relation_get.side_effect = fake_relation_get + self.assertEqual( + cert_utils.get_requests_for_local_unit(), + [{ + 'ca': 'ROOTCA', + 'certs': { + 'juju-cd4bb3-5.lxd': { + 'cert': 'BASECERT', + 'key': 'BASEKEY'}}, + 'chain': 'MYCHAIN'}] + ) + @mock.patch.object(cert_utils, 'get_requests_for_local_unit') def test_get_bundle_for_cn(self, get_requests_for_local_unit): get_requests_for_local_unit.return_value = [{