From aac2c2a178a86a8b686eb26b553283c0e9ba69f2 Mon Sep 17 00:00:00 2001 From: Paul Goins Date: Tue, 31 Aug 2021 11:11:47 -0700 Subject: [PATCH] Sharing SSH pubkeys across nova-compute apps SSH keys from nova-compute are now shared across all nova-compute charm apps. Closes-Bug: #1468871 Change-Id: Ia142eceff56bb763fcca8ddf5b74b83f84bf3539 --- hooks/nova_cc_hooks.py | 81 ++++++----- unit_tests/test_nova_cc_hooks.py | 229 ++++++++++++++++++++++--------- 2 files changed, 207 insertions(+), 103 deletions(-) diff --git a/hooks/nova_cc_hooks.py b/hooks/nova_cc_hooks.py index 287cf48b..34d14c83 100755 --- a/hooks/nova_cc_hooks.py +++ b/hooks/nova_cc_hooks.py @@ -693,7 +693,8 @@ def cloud_compute_relation_changed(): """ add_hosts_to_cell_when_ready() set_region_on_relation_from_config(rid=None) - update_ssh_keys_and_notify_compute_units(rid=None, unit=None) + update_ssh_key(rid=None, unit=None) + update_compute_units_with_ssh_key_change(rid=None, unit=None) def add_hosts_to_cell_when_ready(): @@ -732,16 +733,13 @@ def set_cross_az_attach_on_relation_from_config(rid=None): cross_az_attach=hookenv.config('cross-az-attach')) -def update_ssh_keys_and_notify_compute_units(rid=None, unit=None): - """Update and notify the collected ssh keys to nova-compute units +def update_compute_units_with_ssh_key_change(rid=None, unit=None): + """Notify the updated ssh keys to nova-compute units - Update/add and notify, for the associated nova-compute unit, the ssh key to - all the other nova-compute units. + Notify all associated nova-compute units of the relation. If rid=None and unit=None, then this function is being called in the - context of a cloud-compute relation changed hook, and will relate to that - unit. If rid and unit are set, then this function is being called to - refresh and update all specific units. + context of a cloud-compute relation changed hook. :param rid: The relation to check/set, or if None, the current one related to the hook. @@ -750,13 +748,21 @@ def update_ssh_keys_and_notify_compute_units(rid=None, unit=None): the hook. :type unit: Union[str, None] """ - update_ssh_key(rid=rid, unit=unit) # if we have goal state, then only notify the ssh authorized_keys and # known_hosts onto the relation when the last compute unit has arrived # (i.e. we've reached the goal state) - if _goal_state_achieved_for_relid('cloud-compute', rid): - notify_ssh_keys_to_compute_units(rid=rid, unit=unit) + rids = hookenv.relation_ids('cloud-compute') + if all(_goal_state_achieved_for_relid('cloud-compute', rid) + for rid in rids): + # NOTE(ganso): we are either in a relation-changed hook of a specific + # unit or already in a loop from upgrade-charm + if rid is None and unit is None: + for rid in rids: + for unit in hookenv.related_units(rid): + notify_ssh_keys_to_compute_units(rid=rid, unit=unit) + else: + notify_ssh_keys_to_compute_units(rid=rid, unit=unit) def _goal_state_achieved_for_relid(reltype, rid=None): @@ -885,22 +891,14 @@ def update_ssh_key(rid=None, unit=None): def notify_ssh_keys_to_compute_units(rid=None, unit=None): - """Update and notify the collected ssh keys to nova-compute units + """Notify the updated ssh keys to nova-compute units - Update/add and notify, for the associated nova-compute unit, the ssh key to - all the other nova-compute units. + Notify all associated nova-compute units of the relation. - If rid=None and unit=None, then this function is being called in the - context of a cloud-compute relation changed hook, and will relate to that - unit. If rid and unit are set, then this function is being called to - refresh and update all specific units. - - :param rid: The relation to check/set, or if None, the current one related - to the hook. - :type rid: Union[str. None] - :param unit: the unit to check, of None for the current one according to - the hook. - :type unit: Union[str, None] + :param rid: The relation to check/set. + :type rid: str + :param unit: the unit to check + :type unit: str """ rel_settings = hookenv.relation_get(rid=rid, unit=unit) @@ -908,24 +906,27 @@ def notify_ssh_keys_to_compute_units(rid=None, unit=None): if migration_auth_type is None: return - remote_service = ncc_utils.remote_service_from_unit(unit) + # We distribute the ssh keys of all nova-compute units from all + # nova-compute charm apps + remote_services = set(dir_.split('_')[0] + for dir_ in os.listdir(ncc_utils.NOVA_SSH_DIR)) if migration_auth_type == 'ssh': - _set_hosts_and_keys_on_relation(remote_service, rid, user=None) + _set_hosts_and_keys_on_relation(remote_services, rid, user=None) if rel_settings.get('nova_ssh_public_key', None): - _set_hosts_and_keys_on_relation(remote_service, rid, user='nova') + _set_hosts_and_keys_on_relation(remote_services, rid, user='nova') -def _set_hosts_and_keys_on_relation(remote_service, rid=None, user=None): +def _set_hosts_and_keys_on_relation(remote_services, rid=None, user=None): """Set the known hosts and authorized keys on the relation specified. Takes the authorized_keys and known hosts collected from all of the related compute units (that have been processed) and sets them on the relation via the _batch_write_ssh_on_relation() helper. - :param remote_service: the remote service related the keys/hosts - :type remote_service: str + :param remote_services: the remote service related the keys/hosts + :type remote_services: list[str] :param rid: The relation to check/set, or if None, the current one related to the hook. :type rid: Union[str. None] @@ -946,11 +947,13 @@ def _set_hosts_and_keys_on_relation(remote_service, rid=None, user=None): _batch_write_ssh_on_relation( rid, known_hosts_prefix, known_hosts_max_key, - ncc_utils.ssh_known_hosts_lines(remote_service, user=user)) + [line for svc in remote_services + for line in ncc_utils.ssh_known_hosts_lines(svc, user=user)]) _batch_write_ssh_on_relation( rid, authorized_prefix, authorized_keys_max_key, - ncc_utils.ssh_authorized_keys_lines(remote_service, user=user)) + [line for svc in remote_services + for line in ncc_utils.ssh_authorized_keys_lines(svc, user=user)]) def _batch_write_ssh_on_relation(rid, prefix, max_index, _iter): @@ -1180,16 +1183,22 @@ def upgrade_charm(): # configurations installed at the same time. ncc_utils.stop_deprecated_services() ncc_utils.disable_package_apache_site(service_reload=True) - for r_id in hookenv.relation_ids('amqp'): amqp_joined(relation_id=r_id) for r_id in hookenv.relation_ids('identity-service'): identity_joined(rid=r_id) - for r_id in hookenv.relation_ids('cloud-compute'): + + r_ids = hookenv.relation_ids('cloud-compute') + # NOTE(ganso): we update ssh data for all units first + for r_id in r_ids: + for unit in hookenv.related_units(r_id): + update_ssh_key(rid=r_id, unit=unit) + + for r_id in r_ids: set_region_on_relation_from_config(r_id) set_cross_az_attach_on_relation_from_config(r_id) for unit in hookenv.related_units(r_id): - update_ssh_keys_and_notify_compute_units(r_id, unit) + update_compute_units_with_ssh_key_change(rid=r_id, unit=unit) for r_id in hookenv.relation_ids('shared-db'): db_joined(relation_id=r_id) diff --git a/unit_tests/test_nova_cc_hooks.py b/unit_tests/test_nova_cc_hooks.py index b5415cd0..a02f493f 100644 --- a/unit_tests/test_nova_cc_hooks.py +++ b/unit_tests/test_nova_cc_hooks.py @@ -364,17 +364,20 @@ class NovaCCHooksTests(CharmTestCase): @patch.object(hooks, 'add_hosts_to_cell_when_ready') @patch.object(hooks, 'set_region_on_relation_from_config') - @patch.object(hooks, 'update_ssh_keys_and_notify_compute_units') + @patch.object(hooks, 'update_ssh_key') + @patch.object(hooks, 'update_compute_units_with_ssh_key_change') def test_cloud_compute_relation_changed( self, - mock_update_ssh_keys_and_notify_compute_units, + mock_update_compute_units_with_ssh_key_change, + mock_update_ssh_key, mock_set_region_on_relation_from_config, mock_add_hosts_to_cell_when_ready): hooks.cloud_compute_relation_changed() mock_add_hosts_to_cell_when_ready.assert_called_once_with() mock_set_region_on_relation_from_config.assert_called_once_with( rid=None) - mock_update_ssh_keys_and_notify_compute_units.assert_called_once_with( + mock_update_ssh_key.assert_called_once_with(rid=None, unit=None) + mock_update_compute_units_with_ssh_key_change.assert_called_once_with( rid=None, unit=None) @patch('charmhelpers.core.hookenv.related_units') @@ -445,103 +448,195 @@ class NovaCCHooksTests(CharmTestCase): @patch('charmhelpers.core.unitdata.Storage.set') @patch('hooks.nova_cc_utils.add_authorized_key_if_doesnt_exist') @patch('hooks.nova_cc_utils.ssh_compute_add_known_hosts') - @patch('hooks.nova_cc_hooks._goal_state_achieved_for_relid') @patch('hooks.nova_cc_utils.remote_service_from_unit') - def test_update_ssh_keys_and_notify_compute_units_ssh_migration( + def test_update_ssh_key( self, mock_remote_service_from_unit, - mock__goal_state_achieved_for_relid, mock_ssh_compute_add_known_hosts, mock_add_authorized_key_if_doesnt_exist, mock_db_set, mock_get_hostname): mock_get_hostname.return_value = None mock_remote_service_from_unit.return_value = 'aservice' - mock__goal_state_achieved_for_relid.return_value = True - self.test_relation.set({ + + self.relation_get.side_effect = [{ 'migration_auth_type': 'ssh', 'ssh_public_key': 'fookey', - 'private-address': '10.0.0.1', 'region': 'RegionOne'}) - self.ssh_known_hosts_lines.return_value = [ - 'k_h_0', 'k_h_1', 'k_h_2'] - self.ssh_authorized_keys_lines.return_value = [ - 'auth_0', 'auth_1', 'auth_2'] - hooks.update_ssh_keys_and_notify_compute_units() - mock_db_set.assert_called_once_with('hostset-10.0.0.1', ['10.0.0.1']) + 'private-address': '10.0.0.1', 'region': 'RegionOne'}] + + hooks.update_ssh_key(rid=None, unit=None) mock_ssh_compute_add_known_hosts.assert_called_once_with( 'aservice', ['10.0.0.1'], user=None) mock_add_authorized_key_if_doesnt_exist.assert_called_once_with( 'fookey', 'aservice', '10.0.0.1', user=None) - expected_relations = [ - call(relation_settings={'authorized_keys_0': 'auth_0'}, - relation_id=None), - call(relation_settings={'authorized_keys_1': 'auth_1'}, - relation_id=None), - call(relation_settings={'authorized_keys_2': 'auth_2'}, - relation_id=None), - call(relation_settings={'known_hosts_0': 'k_h_0'}, - relation_id=None), - call(relation_settings={'known_hosts_1': 'k_h_1'}, - relation_id=None), - call(relation_settings={'known_hosts_2': 'k_h_2'}, - relation_id=None), - call(relation_settings={'known_hosts_max_index': 3}, - relation_id=None), - call(relation_settings={'authorized_keys_max_index': 3}, - relation_id=None)] - self.relation_set.assert_has_calls(expected_relations, any_order=True) - mock__goal_state_achieved_for_relid.assert_called_once_with( - 'cloud-compute', None) + mock_db_set.assert_called_once_with('hostset-10.0.0.1', ['10.0.0.1']) @patch('charmhelpers.contrib.openstack.utils.get_hostname') @patch('charmhelpers.core.unitdata.Storage.set') @patch('hooks.nova_cc_utils.add_authorized_key_if_doesnt_exist') @patch('hooks.nova_cc_utils.ssh_compute_add_known_hosts') - @patch('hooks.nova_cc_hooks._goal_state_achieved_for_relid') @patch('hooks.nova_cc_utils.remote_service_from_unit') - def test_update_ssh_keys_and_notify_compute_units_nova_public_key( + def test_update_ssh_key_nova_user( self, mock_remote_service_from_unit, - mock__goal_state_achieved_for_relid, mock_ssh_compute_add_known_hosts, mock_add_authorized_key_if_doesnt_exist, mock_db_set, mock_get_hostname): mock_get_hostname.return_value = None mock_remote_service_from_unit.return_value = 'aservice' - mock__goal_state_achieved_for_relid.return_value = True - self.test_relation.set({ + + self.relation_get.side_effect = [{ 'migration_auth_type': 'sasl', 'nova_ssh_public_key': 'fookey', - 'private-address': '10.0.0.1', 'region': 'RegionOne'}) - self.ssh_known_hosts_lines.return_value = [ - 'k_h_0', 'k_h_1', 'k_h_2'] - self.ssh_authorized_keys_lines.return_value = [ - 'auth_0', 'auth_1', 'auth_2'] - hooks.update_ssh_keys_and_notify_compute_units() - mock_db_set.assert_called_once_with('hostset-10.0.0.1', ['10.0.0.1']) + 'private-address': '10.0.0.1', 'region': 'RegionOne'}] + + hooks.update_ssh_key(rid=None, unit=None) mock_ssh_compute_add_known_hosts.assert_called_once_with( 'aservice', ['10.0.0.1'], user='nova') mock_add_authorized_key_if_doesnt_exist.assert_called_once_with( 'fookey', 'aservice', '10.0.0.1', user='nova') - expected_relations = [ - call(relation_settings={'nova_authorized_keys_0': 'auth_0'}, - relation_id=None), - call(relation_settings={'nova_authorized_keys_1': 'auth_1'}, - relation_id=None), - call(relation_settings={'nova_authorized_keys_2': 'auth_2'}, - relation_id=None), - call(relation_settings={'nova_known_hosts_0': 'k_h_0'}, - relation_id=None), - call(relation_settings={'nova_known_hosts_1': 'k_h_1'}, - relation_id=None), - call(relation_settings={'nova_known_hosts_2': 'k_h_2'}, - relation_id=None), - call(relation_settings={'nova_known_hosts_max_index': 3}, - relation_id=None), - call(relation_settings={'nova_authorized_keys_max_index': 3}, - relation_id=None)] - self.relation_set.assert_has_calls(expected_relations, any_order=True) - mock__goal_state_achieved_for_relid.assert_called_once_with( - 'cloud-compute', None) + mock_db_set.assert_called_once_with('hostset-10.0.0.1', ['10.0.0.1']) + + @patch('os.listdir') + def test_notify_ssh_keys_to_compute_units(self, mock_listdir): + self.relation_ids.return_value = [ + 'cloud-compute:15', 'cloud-compute:16'] + + mock_listdir.return_value = ['nova-compute1', 'nova-compute2'] + + self.relation_get.side_effect = None + self.relation_get.return_value = { + 'migration_auth_type': 'ssh', 'ssh_public_key': 'fookey', + 'private-address': '10.0.0.1', 'region': 'RegionOne'} + + self.ssh_known_hosts_lines.side_effect = [ + ['1k_h_0', '1k_h_1'], ['2k_h_0', '2k_h_1'], + ['1k_h_0', '1k_h_1'], ['2k_h_0', '2k_h_1'], + ] + self.ssh_authorized_keys_lines.side_effect = [ + ['1auth_0', '1auth_1'], ['2auth_0', '2auth_1'], + ['1auth_0', '1auth_1'], ['2auth_0', '2auth_1'], + ] + + hooks.notify_ssh_keys_to_compute_units('cloud-compute:X', 'cmp/X') + + self.relation_set.assert_has_calls([ + call(relation_id='cloud-compute:X', + relation_settings={'known_hosts_0': '1k_h_0'}), + call(relation_id='cloud-compute:X', + relation_settings={'known_hosts_1': '1k_h_1'}), + call(relation_id='cloud-compute:X', + relation_settings={'known_hosts_2': '2k_h_0'}), + call(relation_id='cloud-compute:X', + relation_settings={'known_hosts_3': '2k_h_1'}), + call(relation_id='cloud-compute:X', + relation_settings={'known_hosts_max_index': 4}), + call(relation_id='cloud-compute:X', + relation_settings={'authorized_keys_0': '1auth_0'}), + call(relation_id='cloud-compute:X', + relation_settings={'authorized_keys_1': '1auth_1'}), + call(relation_id='cloud-compute:X', + relation_settings={'authorized_keys_2': '2auth_0'}), + call(relation_id='cloud-compute:X', + relation_settings={'authorized_keys_3': '2auth_1'}), + call(relation_id='cloud-compute:X', + relation_settings={'authorized_keys_max_index': 4}), + ]) + + @patch('os.listdir') + def test_notify_ssh_keys_to_compute_units_nova_user(self, mock_listdir): + self.relation_ids.return_value = [ + 'cloud-compute:15', 'cloud-compute:16'] + + mock_listdir.return_value = ['nova-compute1', 'nova-compute2'] + + self.relation_get.side_effect = None + self.relation_get.return_value = { + 'migration_auth_type': 'sasl', 'nova_ssh_public_key': 'fookey', + 'private-address': '10.0.0.1', 'region': 'RegionOne'} + + self.ssh_known_hosts_lines.side_effect = [ + ['1k_h_0', '1k_h_1'], ['2k_h_0', '2k_h_1'], + ['1k_h_0', '1k_h_1'], ['2k_h_0', '2k_h_1'], + ] + self.ssh_authorized_keys_lines.side_effect = [ + ['1auth_0', '1auth_1'], ['2auth_0', '2auth_1'], + ['1auth_0', '1auth_1'], ['2auth_0', '2auth_1'], + ] + + hooks.notify_ssh_keys_to_compute_units('cloud-compute:X', 'cmp/X') + + self.relation_set.assert_has_calls([ + call(relation_id='cloud-compute:X', + relation_settings={'nova_known_hosts_0': '1k_h_0'}), + call(relation_id='cloud-compute:X', + relation_settings={'nova_known_hosts_1': '1k_h_1'}), + call(relation_id='cloud-compute:X', + relation_settings={'nova_known_hosts_2': '2k_h_0'}), + call(relation_id='cloud-compute:X', + relation_settings={'nova_known_hosts_3': '2k_h_1'}), + call(relation_id='cloud-compute:X', + relation_settings={'nova_known_hosts_max_index': 4}), + call(relation_id='cloud-compute:X', + relation_settings={'nova_authorized_keys_0': '1auth_0'}), + call(relation_id='cloud-compute:X', + relation_settings={'nova_authorized_keys_1': '1auth_1'}), + call(relation_id='cloud-compute:X', + relation_settings={'nova_authorized_keys_2': '2auth_0'}), + call(relation_id='cloud-compute:X', + relation_settings={'nova_authorized_keys_3': '2auth_1'}), + call(relation_id='cloud-compute:X', + relation_settings={'nova_authorized_keys_max_index': 4}), + ]) + + @patch('hooks.nova_cc_hooks._goal_state_achieved_for_relid') + @patch('hooks.nova_cc_hooks.notify_ssh_keys_to_compute_units') + def test_update_compute_units_with_ssh_key_change_rel_changed( + self, + mock_notify_ssh_keys_to_compute_units, + mock__goal_state_achieved_for_relid): + + self.relation_ids.return_value = [ + 'cloud-compute:15', 'cloud-compute:16'] + mock__goal_state_achieved_for_relid.side_effect = [True, True] + + self.related_units.side_effect = [['cmp/0', 'cmp/1'], + ['cmp2/0', 'cmp2/1']] + + hooks.update_compute_units_with_ssh_key_change() + + mock__goal_state_achieved_for_relid.assert_has_calls([ + call('cloud-compute', 'cloud-compute:15'), + call('cloud-compute', 'cloud-compute:16'), + ]) + + mock_notify_ssh_keys_to_compute_units.assert_has_calls([ + call(rid='cloud-compute:15', unit='cmp/0'), + call(rid='cloud-compute:15', unit='cmp/1'), + call(rid='cloud-compute:16', unit='cmp2/0'), + call(rid='cloud-compute:16', unit='cmp2/1'), + ]) + + @patch('hooks.nova_cc_hooks._goal_state_achieved_for_relid') + @patch('hooks.nova_cc_hooks.notify_ssh_keys_to_compute_units') + def test_update_compute_units_with_ssh_key_change_upgrade_charm( + self, + mock_notify_ssh_keys_to_compute_units, + mock__goal_state_achieved_for_relid): + + self.relation_ids.return_value = [ + 'cloud-compute:15', 'cloud-compute:16'] + mock__goal_state_achieved_for_relid.side_effect = [True, True] + + hooks.update_compute_units_with_ssh_key_change( + rid='cloud-compute:X', unit='cmp/X') + + mock__goal_state_achieved_for_relid.assert_has_calls([ + call('cloud-compute', 'cloud-compute:15'), + call('cloud-compute', 'cloud-compute:16'), + ]) + + mock_notify_ssh_keys_to_compute_units.assert_called_once_with( + rid='cloud-compute:X', unit='cmp/X') @patch('hooks.nova_cc_utils.is_cellv2_init_ready') @patch('hooks.nova_cc_utils.is_db_initialised')