From 0ecf0015b2a9b7f1340346ab5329d3554e8a221c Mon Sep 17 00:00:00 2001 From: Louis Bouchard Date: Fri, 2 May 2014 18:10:17 +0200 Subject: [PATCH 1/4] Functional line by line fix to the charm. Need more testing for the nova user --- hooks/nova_compute_utils.py | 43 +++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/hooks/nova_compute_utils.py b/hooks/nova_compute_utils.py index 0357e125..03cd2cb9 100644 --- a/hooks/nova_compute_utils.py +++ b/hooks/nova_compute_utils.py @@ -338,26 +338,47 @@ def import_authorized_keys(user='root', prefix=None): """Import SSH authorized_keys + known_hosts from a cloud-compute relation and store in user's $HOME/.ssh. """ + known_hosts = [] + authorized_keys = [] if prefix: - hosts = relation_get('{}_known_hosts'.format(prefix)) - auth_keys = relation_get('{}_authorized_keys'.format(prefix)) + known_hosts_index = relation_get( + '{}_known_hosts_max_index'.format(prefix)) + if known_hosts_index: + for index in range(0, int(known_hosts_index)): + known_hosts.append(relation_get( + '{}_known_hosts_{}'.format(prefix, index))) + authorized_keys_index = relation_get( + '{}_authorized_keys_max_index'.format(prefix)) + if authorized_keys: + for index in range(0, int(authorized_keys_index)): + authorized_keys.append(relation_get( + '{}_authorized_keys_{}'.format(prefix, index))) else: # XXX: Should this be managed via templates + contexts? - hosts = relation_get('known_hosts') - auth_keys = relation_get('authorized_keys') + known_hosts_index = relation_get('known_hosts_max_index') + if known_hosts_index: + for index in range(0, int(known_hosts_index)): + known_hosts.append(relation_get( + 'known_hosts_{}'.format(index))) + authorized_keys_index = relation_get('authorized_keys_max_index') + if authorized_keys_index: + for index in range(0, int(authorized_keys_index)): + authorized_keys.append(relation_get( + 'authorized_keys_{}'.format(index))) - # XXX: Need to fix charm-helpers to return None for empty settings, - # in all cases. - if not hosts or not auth_keys: + # XXX: Should partial return of known_hosts or authorized_keys + # be allowed ? + if not len(known_hosts) or not len(authorized_keys): return dest = os.path.join(pwd.getpwnam(user).pw_dir, '.ssh') log('Saving new known_hosts and authorized_keys file to: %s.' % dest) - - with open(os.path.join(dest, 'authorized_keys'), 'wb') as _keys: - _keys.write(b64decode(auth_keys)) with open(os.path.join(dest, 'known_hosts'), 'wb') as _hosts: - _hosts.write(b64decode(hosts)) + for index in range(0, int(known_hosts_index)): + _hosts.write(known_hosts[index]) + with open(os.path.join(dest, 'authorized_keys'), 'wb') as _keys: + for index in range(0, int(authorized_keys_index)): + _keys.write(authorized_keys[index]) def do_openstack_upgrade(): From af7678a46dd9257e52eabad5dc66b4408e235407 Mon Sep 17 00:00:00 2001 From: Louis Bouchard Date: Tue, 6 May 2014 15:45:04 +0200 Subject: [PATCH 2/4] Support for nova user added --- hooks/nova_compute_utils.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/hooks/nova_compute_utils.py b/hooks/nova_compute_utils.py index 03cd2cb9..fb8e0dc5 100644 --- a/hooks/nova_compute_utils.py +++ b/hooks/nova_compute_utils.py @@ -349,7 +349,7 @@ def import_authorized_keys(user='root', prefix=None): '{}_known_hosts_{}'.format(prefix, index))) authorized_keys_index = relation_get( '{}_authorized_keys_max_index'.format(prefix)) - if authorized_keys: + if authorized_keys_index: for index in range(0, int(authorized_keys_index)): authorized_keys.append(relation_get( '{}_authorized_keys_{}'.format(prefix, index))) @@ -370,15 +370,14 @@ def import_authorized_keys(user='root', prefix=None): # be allowed ? if not len(known_hosts) or not len(authorized_keys): return - dest = os.path.join(pwd.getpwnam(user).pw_dir, '.ssh') log('Saving new known_hosts and authorized_keys file to: %s.' % dest) with open(os.path.join(dest, 'known_hosts'), 'wb') as _hosts: for index in range(0, int(known_hosts_index)): - _hosts.write(known_hosts[index]) + _hosts.write('{}\n'.format(known_hosts[index])) with open(os.path.join(dest, 'authorized_keys'), 'wb') as _keys: for index in range(0, int(authorized_keys_index)): - _keys.write(authorized_keys[index]) + _keys.write('{}\n'.format(authorized_keys[index])) def do_openstack_upgrade(): From 144d69bf012e7a9feb16b0c33b17f35bec23005a Mon Sep 17 00:00:00 2001 From: Louis Bouchard Date: Tue, 6 May 2014 16:15:33 +0200 Subject: [PATCH 3/4] PEP8 cleanup --- hooks/nova_compute_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hooks/nova_compute_utils.py b/hooks/nova_compute_utils.py index fb8e0dc5..e7e1a63a 100644 --- a/hooks/nova_compute_utils.py +++ b/hooks/nova_compute_utils.py @@ -342,13 +342,13 @@ def import_authorized_keys(user='root', prefix=None): authorized_keys = [] if prefix: known_hosts_index = relation_get( - '{}_known_hosts_max_index'.format(prefix)) + '{}_known_hosts_max_index'.format(prefix)) if known_hosts_index: for index in range(0, int(known_hosts_index)): known_hosts.append(relation_get( '{}_known_hosts_{}'.format(prefix, index))) authorized_keys_index = relation_get( - '{}_authorized_keys_max_index'.format(prefix)) + '{}_authorized_keys_max_index'.format(prefix)) if authorized_keys_index: for index in range(0, int(authorized_keys_index)): authorized_keys.append(relation_get( From 56b86b62c045ef6adc6b5e07dade58a63f74185d Mon Sep 17 00:00:00 2001 From: Louis Bouchard Date: Thu, 22 May 2014 12:46:30 +0200 Subject: [PATCH 4/4] Adapt unit tests to changes made by previous commit Tests now cover multi-line import of known_hosts & authorized_keys --- unit_tests/test_nova_compute_utils.py | 78 +++++++++++++++++++-------- 1 file changed, 56 insertions(+), 22 deletions(-) diff --git a/unit_tests/test_nova_compute_utils.py b/unit_tests/test_nova_compute_utils.py index 0db47141..f8101269 100644 --- a/unit_tests/test_nova_compute_utils.py +++ b/unit_tests/test_nova_compute_utils.py @@ -185,55 +185,89 @@ class NovaComputeUtilsTests(CharmTestCase): def _test_import_authorized_keys_base(self, getpwnam, prefix=None): getpwnam.return_value = self.fake_user('foo') self.relation_get.side_effect = [ - 'Zm9vX2tleQo=', # relation_get('known_hosts') - 'Zm9vX2hvc3QK', # relation_get('authorized_keys') + 3, # relation_get('known_hosts_max_index') + 'k_h_0', # relation_get_('known_hosts_0') + 'k_h_1', # relation_get_('known_hosts_1') + 'k_h_2', # relation_get_('known_hosts_2') + 3, # relation_get('authorized_keys_max_index') + 'auth_0', # relation_get('authorized_keys_0') + 'auth_1', # relation_get('authorized_keys_1') + 'auth_2', # relation_get('authorized_keys_2') ] ex_open = [ - call('/home/foo/.ssh/authorized_keys', 'wb'), - call('/home/foo/.ssh/known_hosts', 'wb') + call('/home/foo/.ssh/known_hosts', 'wb'), + call('/home/foo/.ssh/authorized_keys', 'wb') ] ex_write = [ - call('foo_host\n'), - call('foo_key\n'), + call('k_h_0\n'), + call('k_h_1\n'), + call('k_h_2\n'), + call('auth_0\n'), + call('auth_1\n'), + call('auth_2\n') ] with patch_open() as (_open, _file): utils.import_authorized_keys(user='foo') self.assertEquals(ex_open, _open.call_args_list) self.assertEquals(ex_write, _file.write.call_args_list) - - self.relation_get.assert_has_called([ - call('known_hosts'). - call('authorized_keys') - ]) + expected_relations = [ + call('known_hosts_max_index'), + call('known_hosts_0'), + call('known_hosts_1'), + call('known_hosts_2'), + call('authorized_keys_max_index'), + call('authorized_keys_0'), + call('authorized_keys_1'), + call('authorized_keys_2') + ] + self.assertEquals(sorted(self.relation_get.call_args_list), + sorted(expected_relations)) @patch('pwd.getpwnam') def test_import_authorized_keys_prefix(self, getpwnam): getpwnam.return_value = self.fake_user('foo') self.relation_get.side_effect = [ - 'Zm9vX2tleQo=', # relation_get('known_hosts') - 'Zm9vX2hvc3QK', # relation_get('authorized_keys') + 3, # relation_get('bar_known_hosts_max_index') + 'k_h_0', # relation_get_('bar_known_hosts_0') + 'k_h_1', # relation_get_('bar_known_hosts_1') + 'k_h_2', # relation_get_('bar_known_hosts_2') + 3, # relation_get('bar_authorized_keys_max_index') + 'auth_0', # relation_get('bar_authorized_keys_0') + 'auth_1', # relation_get('bar_authorized_keys_1') + 'auth_2', # relation_get('bar_authorized_keys_2') ] ex_open = [ - call('/home/foo/.ssh/authorized_keys', 'wb'), - call('/home/foo/.ssh/known_hosts', 'wb') + call('/home/foo/.ssh/known_hosts', 'wb'), + call('/home/foo/.ssh/authorized_keys', 'wb') ] ex_write = [ - call('foo_host\n'), - call('foo_key\n'), + call('k_h_0\n'), + call('k_h_1\n'), + call('k_h_2\n'), + call('auth_0\n'), + call('auth_1\n'), + call('auth_2\n') ] with patch_open() as (_open, _file): utils.import_authorized_keys(user='foo', prefix='bar') self.assertEquals(ex_open, _open.call_args_list) self.assertEquals(ex_write, _file.write.call_args_list) - - self.relation_get.assert_has_called([ - call('bar_known_hosts'). - call('bar_authorized_keys') - ]) + expected_relations = [ + call('bar_known_hosts_max_index'), + call('bar_known_hosts_0'), + call('bar_known_hosts_1'), + call('bar_known_hosts_2'), + call('bar_authorized_keys_max_index'), + call('bar_authorized_keys_0'), + call('bar_authorized_keys_1'), + call('bar_authorized_keys_2') + ] + self.assertEquals(sorted(self.relation_get.call_args_list), + sorted(expected_relations)) @patch('subprocess.check_call') def test_import_keystone_cert_missing_data(self, check_call):