openstack
/
charm-nova-cloud-controller
Code Issues Proposed changes

Fix ssh keys duplication 

Upon running hooks that update ssh_keys, they
end up duplicated in the /etc/nova/compute_ssh/* files
and cloud-compute relations because the code that
checks whether the keys already exist are currently
not working.

This change fixes the deduplication code and improves
unit tests, while also handling a special case for
specific ubuntu-version scenarios.

Change-Id: I93f9418d5340e7cb599a42970d78557362c1542f
Closes-bug: #1943753
This commit is contained in:
Rodrigo Barbieri 2021-09-15 16:27:14 -03:00 committed by Rodrigo Barbieri
parent fc33f99c5f
commit 1a63d9c0b6
2 changed files with 70 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
hooks/nova_cc_utils.py
View File

@ -1162,18 +1162,26 @@ def ssh_known_host_key(host, remote_service, user=None):
"""
cmd = ['ssh-keygen', '-f',
known_hosts(remote_service, user), '-H', '-F', host]
try:
# The first line of output is like '# Host xx found: line 1 type RSA',
# which should be excluded.
output = subprocess.check_output(cmd).decode('utf-8').strip()
except subprocess.CalledProcessError:
return None
except subprocess.CalledProcessError as e:
# NOTE(ganso): On Xenial and Bionic, RC is always 1 and
# does not include line "Host <host> found: line <number>"
if e.output and not e.stderr:
output = e.output.decode('utf-8').strip()
else:
return None
if output:
# Bug #1500589 cmd has 0 rc on precise if entry not present
lines = output.split('\n')
if len(lines) > 1:
return lines[1]
if len(lines) > 0:
# NOTE(ganso): On Focal, RC is 0 if success
# and includes the line "Host <host> found: line <number>"
if "found: line" in lines[0]:
return lines[1]
return lines[0]
return None
@ -1266,7 +1274,7 @@ def ssh_authorized_key_exists(public_key, remote_service, user=None):
:returns: True if the key is in the specified authorized key file
"""
with open(authorized_keys(remote_service, user)) as keys:
return ' {} '.format(public_key) in keys.read()
return public_key in keys.read()
def add_authorized_key_if_doesnt_exist(public_key,

58
unit_tests/test_nova_cc_utils.py
View File

@ -14,6 +14,7 @@
from collections import OrderedDict
from mock import patch, MagicMock, call
import subprocess
from unit_tests.test_utils import (
CharmTestCase,
@ -808,7 +809,8 @@ class NovaCCUtilsTests(CharmTestCase):
@patch.object(utils, 'authorized_keys')
def test_ssh_authorized_key_exists(self, keys):
key = 'BBBBB3NzaC1yc2EBBBBDBQBBBBBBBQC27Us7lSjCpa7bumXBgc'
key = 'ssh-rsa BBBBB3NzaC1yc2EBBBBDBQBBBBBBBQC27Us7lSjCpa7bumXBgc' \
' nova-compute-2'
with patch_open() as (_open, _file):
_file.read.return_value = AUTHORIZED_KEYS
self.assertTrue(utils.ssh_authorized_key_exists(key, 'aservice'))
@ -879,9 +881,11 @@ class NovaCCUtilsTests(CharmTestCase):
@patch.object(utils, 'known_hosts')
@patch('subprocess.check_output')
def test_ssh_known_host_key(self, _check_output, _known_hosts):
def test_ssh_known_host_key_rc0(self, _check_output, _known_hosts):
_known_hosts.return_value = '/foo/known_hosts'
utils.ssh_known_host_key('test', 'aservice')
_check_output.return_value = b"hash1 ssh-rsa key1\nhash2 ssh-rsa key2"
result = utils.ssh_known_host_key('test', 'aservice')
self.assertEqual("hash1 ssh-rsa key1", result)
_check_output.assert_called_with(
['ssh-keygen', '-f', '/foo/known_hosts',
'-H', '-F', 'test'])
@ -899,6 +903,54 @@ class NovaCCUtilsTests(CharmTestCase):
key = utils.ssh_known_host_key('test', 'aservice')
self.assertEqual(key, None)
@patch.object(utils, 'known_hosts')
@patch('subprocess.check_output')
def test_ssh_known_host_key_rc0_header(self, _check_output, _known_hosts):
cmd = ['ssh-keygen', '-f', '/foo/known_hosts',
'-H', '-F', 'test']
_known_hosts.return_value = '/foo/known_hosts'
_check_output.return_value = (b"# Host foo found: line 7\n"
b"hash1 ssh-rsa key1\n"
b"hash2 ssh-rsa key2")
result = utils.ssh_known_host_key('test', 'aservice')
self.assertEqual("hash1 ssh-rsa key1", result)
_check_output.assert_called_with(cmd)
_known_hosts.assert_called_with('aservice', None)
utils.ssh_known_host_key('test', 'bar')
_known_hosts.assert_called_with('bar', None)
@patch.object(utils, 'known_hosts')
@patch('subprocess.check_output')
def test_ssh_known_host_key_rc1(self, _check_output, _known_hosts):
cmd = ['ssh-keygen', '-f', '/foo/known_hosts',
'-H', '-F', 'test']
_known_hosts.return_value = '/foo/known_hosts'
_check_output.side_effect = subprocess.CalledProcessError(
1, ' '.join(cmd),
output=b"hash1 ssh-rsa key1\nhash2 ssh-rsa key2", stderr='')
result = utils.ssh_known_host_key('test', 'aservice')
self.assertEqual("hash1 ssh-rsa key1", result)
_check_output.assert_called_with(cmd)
_known_hosts.assert_called_with('aservice', None)
utils.ssh_known_host_key('test', 'bar')
_known_hosts.assert_called_with('bar', None)
@patch.object(utils, 'known_hosts')
@patch('subprocess.check_output')
def test_ssh_known_host_key_rc1_stderr(self, _check_output, _known_hosts):
cmd = ['ssh-keygen', '-f', '/foo/known_hosts',
'-H', '-F', 'test']
_known_hosts.return_value = '/foo/known_hosts'
_check_output.side_effect = subprocess.CalledProcessError(
1, ' '.join(cmd),
output="foobar", stderr='command error')
result = utils.ssh_known_host_key('test', 'aservice')
self.assertIsNone(result)
_check_output.assert_called_with(cmd)
_known_hosts.assert_called_with('aservice', None)
utils.ssh_known_host_key('test', 'bar')
_known_hosts.assert_called_with('bar', None)
@patch.object(utils, 'known_hosts')
@patch('subprocess.check_call')
def test_remove_known_host(self, _check_call, _known_hosts):