openstack/ironic
Code Issues Proposed changes

Fixes parameters validation in SSH power manager.

Browse Source 
Parameters validation was not in conformity with the docs, and it
was not possible to pass a SSH key with a passphrase. This patch
fixes the validation allowing the user to pass both parameters,
and adds tests for this validation.

Change-Id: I1ba1eb8393c7921f4cab154b69b856418af32173
Closes-Bug: #1607527
This commit is contained in:
Ricardo Araújo Santos 2016-09-06 10:47:46 -03:00
parent b1091d9a35
commit 50c7fdc45c
4 changed files with 75 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
ironic/drivers/modules/ssh.py
View File

@ -58,14 +58,18 @@ REQUIRED_PROPERTIES = {
"vmware, parallels, xenserver. Required.")
}
OTHER_PROPERTIES = {
'ssh_key_contents': _("private key(s). One of this, ssh_key_filename, "
"or ssh_password must be specified."),
'ssh_key_contents': _("private key(s). If ssh_password is also specified "
"it will be used for unlocking the private key. Do "
"not specify ssh_key_filename when this property is "
"specified."),
'ssh_key_filename': _("(list of) filename(s) of optional private key(s) "
"for authentication. One of this, ssh_key_contents, "
"or ssh_password must be specified."),
"for authentication. If ssh_password is also "
"specified it will be used for unlocking the "
"private key. Do not specify ssh_key_contents when "
"this property is specified."),
'ssh_password': _("password to use for authentication or for unlocking a "
"private key. One of this, ssh_key_contents, or "
"ssh_key_filename must be specified."),
"private key. At least one of this, ssh_key_contents, "
"or ssh_key_filename must be specified."),
'ssh_port': _("port on the node to connect to; default is 22. Optional."),
'vbox_use_headless': _("True or False (Default). Optional. "
"In the case of VirtualBox 3.2 and above, allows "
@ -392,17 +396,25 @@ def _parse_driver_info(node):
cmd_set = _get_command_sets(virt_type, use_headless)
res['cmd_set'] = cmd_set
# Only one credential may be set (avoids complexity around having
# precedence etc).
if len([v for v in (password, key_filename, key_contents) if v]) != 1:
# Set at least one credential method.
if len([v for v in (password, key_filename, key_contents) if v]) == 0:
raise exception.InvalidParameterValue(_(
"SSHPowerDriver requires one and only one of ssh_password, "
"SSHPowerDriver requires at least one of ssh_password, "
"ssh_key_contents and ssh_key_filename to be set."))
# Set only credential file or content but not both
if key_filename and key_contents:
raise exception.InvalidParameterValue(_(
"SSHPowerDriver requires one and only one of "
"ssh_key_contents and ssh_key_filename to be set."))
if password:
res['password'] = password
elif key_contents:
if key_contents:
res['key_contents'] = key_contents
else:
if key_filename:
if not os.path.isfile(key_filename):
raise exception.InvalidParameterValue(_(
"SSH key file %s not found.") % key_filename)

8
ironic/tests/unit/db/utils.py
View File

@ -53,9 +53,15 @@ def get_test_ssh_info(auth_type='password', virt_type='virsh'):
result['ssh_key_filename'] = '/not/real/file'
elif 'key' == auth_type:
result['ssh_key_contents'] = '--BEGIN PRIVATE ...blah'
elif 'too_many' == auth_type:
elif 'file_with_passphrase' == auth_type:
result['ssh_password'] = 'fake'
result['ssh_key_filename'] = '/not/real/file'
elif 'key_with_passphrase' == auth_type:
result['ssh_password'] = 'fake'
result['ssh_key_contents'] = '--BEGIN PRIVATE ...blah'
elif 'too_many' == auth_type:
result['ssh_key_contents'] = '--BEGIN PRIVATE ...blah'
result['ssh_key_filename'] = '/not/real/file'
else:
# No auth details (is invalid)
pass

42
ironic/tests/unit/drivers/modules/test_ssh.py
View File

@ -96,6 +96,45 @@ class SSHValidateParametersTestCase(db_base.DbTestCase):
self.assertEqual('1be26c0b-03f2-4d2e-ae87-c02d7f33c123',
info['uuid'])
def test__parse_driver_info_good_file_with_passphrase(self):
# make sure we get back the expected things
d_info = db_utils.get_test_ssh_info('file_with_passphrase')
tempdir = tempfile.mkdtemp()
key_path = tempdir + '/foo'
open(key_path, 'wt').close()
d_info['ssh_key_filename'] = key_path
node = obj_utils.get_test_node(
self.context,
driver='fake_ssh',
driver_info=d_info)
info = ssh._parse_driver_info(node)
self.assertEqual('1.2.3.4', info['host'])
self.assertEqual('admin', info['username'])
self.assertEqual('fake', info['password'])
self.assertEqual(key_path, info['key_filename'])
self.assertEqual(22, info['port'])
self.assertEqual('virsh', info['virt_type'])
self.assertIsNotNone(info['cmd_set'])
self.assertEqual('1be26c0b-03f2-4d2e-ae87-c02d7f33c123',
info['uuid'])
def test__parse_driver_info_good_key_with_passphrase(self):
# make sure we get back the expected things
node = obj_utils.get_test_node(
self.context,
driver='fake_ssh',
driver_info=db_utils.get_test_ssh_info('key_with_passphrase'))
info = ssh._parse_driver_info(node)
self.assertEqual('1.2.3.4', info['host'])
self.assertEqual('admin', info['username'])
self.assertEqual('fake', info['password'])
self.assertEqual('--BEGIN PRIVATE ...blah', info['key_contents'])
self.assertEqual(22, info['port'])
self.assertEqual('virsh', info['virt_type'])
self.assertIsNotNone(info['cmd_set'])
self.assertEqual('1be26c0b-03f2-4d2e-ae87-c02d7f33c123',
info['uuid'])
def test__parse_driver_info_bad_file(self):
# A filename that doesn't exist errors.
info = db_utils.get_test_ssh_info('file')
@ -107,11 +146,10 @@ class SSHValidateParametersTestCase(db_base.DbTestCase):
exception.InvalidParameterValue, ssh._parse_driver_info, node)
def test__parse_driver_info_too_many(self):
info = db_utils.get_test_ssh_info('too_many')
node = obj_utils.get_test_node(
self.context,
driver='fake_ssh',
driver_info=info)
driver_info=db_utils.get_test_ssh_info('too_many'))
self.assertRaises(
exception.InvalidParameterValue, ssh._parse_driver_info, node)

4
releasenotes/notes/bug-1607527-75885e145db62d69.yaml Normal file
View File

@ -0,0 +1,4 @@
---
fixes:
- Fixes bug where the user tries to configure the ssh power driver using a
private key protected by a passphrase.