openstack/python-openstackclient
Code Issues Proposed changes

Switch 'openstack keypair' ops to use SDK

Browse Source 
Let's continue our journey and start using SDK for the keypair
operations

Depends-On: https://review.opendev.org/#/c/761883/
Change-Id: Id411e70b8e1a79c0e88a0e22be7ff37e5c30fcda
This commit is contained in:
Artem Goncharov 2020-11-09 13:14:53 +01:00
parent cb6659d7cd
commit 512ba114a1
3 changed files with 128 additions and 146 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
openstackclient/compute/v2/keypair.py
View File

@ -20,7 +20,7 @@ import logging
import os
import sys
from novaclient import api_versions
from openstack import utils as sdk_utils
from osc_lib.command import command
from osc_lib import exceptions
from osc_lib import utils
@ -32,6 +32,19 @@ from openstackclient.identity import common as identity_common
LOG = logging.getLogger(__name__)
def _get_keypair_columns(item, hide_pub_key=False, hide_priv_key=False):
# To maintain backwards compatibility we need to rename sdk props to
# whatever OSC was using before
column_map = {}
hidden_columns = ['links', 'location']
if hide_pub_key:
hidden_columns.append('public_key')
if hide_priv_key:
hidden_columns.append('private_key')
return utils.get_osc_show_columns_for_sdk_resource(
item, column_map, hidden_columns)
class CreateKeypair(command.ShowOne):
_description = _("Create new public or private key for server ssh access")
@ -76,9 +89,13 @@ class CreateKeypair(command.ShowOne):
return parser
def take_action(self, parsed_args):
compute_client = self.app.client_manager.compute
compute_client = self.app.client_manager.sdk_connection.compute
identity_client = self.app.client_manager.identity
kwargs = {
'name': parsed_args.name
}
public_key = parsed_args.public_key
if public_key:
try:
@ -91,12 +108,10 @@ class CreateKeypair(command.ShowOne):
"exception": e}
)
kwargs = {
'name': parsed_args.name,
'public_key': public_key,
}
kwargs['public_key'] = public_key
if parsed_args.type:
if compute_client.api_version < api_versions.APIVersion('2.2'):
if not sdk_utils.supports_microversion(compute_client, '2.2'):
msg = _(
'--os-compute-api-version 2.2 or greater is required to '
'support the --type option'
@ -106,7 +121,7 @@ class CreateKeypair(command.ShowOne):
kwargs['key_type'] = parsed_args.type
if parsed_args.user:
if compute_client.api_version < api_versions.APIVersion('2.10'):
if not sdk_utils.supports_microversion(compute_client, '2.10'):
msg = _(
'--os-compute-api-version 2.10 or greater is required to '
'support the --user option'
@ -119,7 +134,7 @@ class CreateKeypair(command.ShowOne):
parsed_args.user_domain,
).id
keypair = compute_client.keypairs.create(**kwargs)
keypair = compute_client.create_keypair(**kwargs)
private_key = parsed_args.private_key
# Save private key into specified file
@ -139,14 +154,12 @@ class CreateKeypair(command.ShowOne):
# NOTE(dtroyer): how do we want to handle the display of the private
# key when it needs to be communicated back to the user
# For now, duplicate nova keypair-add command output
info = {}
if public_key or private_key:
info.update(keypair._info)
if 'public_key' in info:
del info['public_key']
if 'private_key' in info:
del info['private_key']
return zip(*sorted(info.items()))
display_columns, columns = _get_keypair_columns(
keypair, hide_pub_key=True, hide_priv_key=True)
data = utils.get_item_properties(keypair, columns)
return (display_columns, data)
else:
sys.stdout.write(keypair.private_key)
return ({}, {})
@ -175,14 +188,14 @@ class DeleteKeypair(command.Command):
return parser
def take_action(self, parsed_args):
compute_client = self.app.client_manager.compute
compute_client = self.app.client_manager.sdk_connection.compute
identity_client = self.app.client_manager.identity
kwargs = {}
result = 0
if parsed_args.user:
if compute_client.api_version < api_versions.APIVersion('2.10'):
if not sdk_utils.supports_microversion(compute_client, '2.10'):
msg = _(
'--os-compute-api-version 2.10 or greater is required to '
'support the --user option'
@ -197,9 +210,8 @@ class DeleteKeypair(command.Command):
for n in parsed_args.name:
try:
data = utils.find_resource(
compute_client.keypairs, n)
compute_client.keypairs.delete(data.name, **kwargs)
compute_client.delete_keypair(
n, **kwargs, ignore_missing=False)
except Exception as e:
result += 1
LOG.error(_("Failed to delete key with name "
@ -240,11 +252,11 @@ class ListKeypair(command.Lister):
return parser
def take_action(self, parsed_args):
compute_client = self.app.client_manager.compute
compute_client = self.app.client_manager.sdk_connection.compute
identity_client = self.app.client_manager.identity
if parsed_args.project:
if compute_client.api_version < api_versions.APIVersion('2.10'):
if not sdk_utils.supports_microversion(compute_client, '2.10'):
msg = _(
'--os-compute-api-version 2.10 or greater is required to '
'support the --project option'
@ -263,9 +275,9 @@ class ListKeypair(command.Lister):
data = []
for user in users:
data.extend(compute_client.keypairs.list(user_id=user.id))
data.extend(compute_client.keypairs(user_id=user.id))
elif parsed_args.user:
if compute_client.api_version < api_versions.APIVersion('2.10'):
if not sdk_utils.supports_microversion(compute_client, '2.10'):
msg = _(
'--os-compute-api-version 2.10 or greater is required to '
'support the --user option'
@ -278,16 +290,16 @@ class ListKeypair(command.Lister):
parsed_args.user_domain,
)
data = compute_client.keypairs.list(user_id=user.id)
data = compute_client.keypairs(user_id=user.id)
else:
data = compute_client.keypairs.list()
data = compute_client.keypairs()
columns = (
"Name",
"Fingerprint"
)
if compute_client.api_version >= api_versions.APIVersion('2.2'):
if sdk_utils.supports_microversion(compute_client, '2.2'):
columns += ("Type", )
return (
@ -324,13 +336,13 @@ class ShowKeypair(command.ShowOne):
return parser
def take_action(self, parsed_args):
compute_client = self.app.client_manager.compute
compute_client = self.app.client_manager.sdk_connection.compute
identity_client = self.app.client_manager.identity
kwargs = {}
if parsed_args.user:
if compute_client.api_version < api_versions.APIVersion('2.10'):
if not sdk_utils.supports_microversion(compute_client, '2.10'):
msg = _(
'--os-compute-api-version 2.10 or greater is required to '
'support the --user option'
@ -343,16 +355,14 @@ class ShowKeypair(command.ShowOne):
parsed_args.user_domain,
).id
keypair = utils.find_resource(
compute_client.keypairs, parsed_args.name, **kwargs)
keypair = compute_client.find_keypair(
parsed_args.name, **kwargs, ignore_missing=False)
info = {}
info.update(keypair._info)
if not parsed_args.public_key:
del info['public_key']
return zip(*sorted(info.items()))
display_columns, columns = _get_keypair_columns(
keypair, hide_pub_key=True)
data = utils.get_item_properties(keypair, columns)
return (display_columns, data)
else:
# NOTE(dtroyer): a way to get the public key in a similar form
# as the private key in the create command
sys.stdout.write(keypair.public_key)
return ({}, {})

175
openstackclient/tests/unit/compute/v2/test_keypair.py
View File

@ -19,8 +19,8 @@ from unittest.mock import call
import uuid
from novaclient import api_versions
from openstack import utils as sdk_utils
from osc_lib import exceptions
from osc_lib import utils
from openstackclient.compute.v2 import keypair
from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes
@ -34,10 +34,6 @@ class TestKeypair(compute_fakes.TestComputev2):
def setUp(self):
super(TestKeypair, self).setUp()
# Get a shortcut to the KeypairManager Mock
self.keypairs_mock = self.app.client_manager.compute.keypairs
self.keypairs_mock.reset_mock()
# Initialize the user mock
self.users_mock = self.app.client_manager.identity.users
self.users_mock.reset_mock()
@ -47,6 +43,14 @@ class TestKeypair(compute_fakes.TestComputev2):
loaded=True,
)
self.app.client_manager.sdk_connection = mock.Mock()
self.app.client_manager.sdk_connection.compute = mock.Mock()
self.sdk_client = self.app.client_manager.sdk_connection.compute
self.sdk_client.keypairs = mock.Mock()
self.sdk_client.create_keypair = mock.Mock()
self.sdk_client.delete_keypair = mock.Mock()
self.sdk_client.find_keypair = mock.Mock()
class TestKeypairCreate(TestKeypair):
@ -71,7 +75,7 @@ class TestKeypairCreate(TestKeypair):
# Get the command object to test
self.cmd = keypair.CreateKeypair(self.app, None)
self.keypairs_mock.create.return_value = self.keypair
self.sdk_client.create_keypair.return_value = self.keypair
def test_key_pair_create_no_options(self):
@ -85,9 +89,8 @@ class TestKeypairCreate(TestKeypair):
columns, data = self.cmd.take_action(parsed_args)
self.keypairs_mock.create.assert_called_with(
name=self.keypair.name,
public_key=None
self.sdk_client.create_keypair.assert_called_with(
name=self.keypair.name
)
self.assertEqual({}, columns)
@ -97,7 +100,7 @@ class TestKeypairCreate(TestKeypair):
# overwrite the setup one because we want to omit private_key
self.keypair = compute_fakes.FakeKeypair.create_one_keypair(
no_pri=True)
self.keypairs_mock.create.return_value = self.keypair
self.sdk_client.create_keypair.return_value = self.keypair
self.data = (
self.keypair.fingerprint,
@ -124,7 +127,7 @@ class TestKeypairCreate(TestKeypair):
columns, data = self.cmd.take_action(parsed_args)
self.keypairs_mock.create.assert_called_with(
self.sdk_client.create_keypair.assert_called_with(
name=self.keypair.name,
public_key=self.keypair.public_key,
)
@ -151,9 +154,8 @@ class TestKeypairCreate(TestKeypair):
columns, data = self.cmd.take_action(parsed_args)
self.keypairs_mock.create.assert_called_with(
self.sdk_client.create_keypair.assert_called_with(
name=self.keypair.name,
public_key=None,
)
mock_open.assert_called_once_with(tmp_pk_file, 'w+')
@ -162,14 +164,12 @@ class TestKeypairCreate(TestKeypair):
self.assertEqual(self.columns, columns)
self.assertEqual(self.data, data)
def test_keypair_create_with_key_type(self):
self.app.client_manager.compute.api_version = api_versions.APIVersion(
'2.2')
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=True)
def test_keypair_create_with_key_type(self, sm_mock):
for key_type in ['x509', 'ssh']:
self.keypair = compute_fakes.FakeKeypair.create_one_keypair(
no_pri=True)
self.keypairs_mock.create.return_value = self.keypair
self.sdk_client.create_keypair.return_value = self.keypair
self.data = (
self.keypair.fingerprint,
@ -195,7 +195,7 @@ class TestKeypairCreate(TestKeypair):
m_file.read.return_value = 'dummy'
columns, data = self.cmd.take_action(parsed_args)
self.keypairs_mock.create.assert_called_with(
self.sdk_client.create_keypair.assert_called_with(
name=self.keypair.name,
public_key=self.keypair.public_key,
key_type=key_type,
@ -204,10 +204,8 @@ class TestKeypairCreate(TestKeypair):
self.assertEqual(self.columns, columns)
self.assertEqual(self.data, data)
def test_keypair_create_with_key_type_pre_v22(self):
self.app.client_manager.compute.api_version = api_versions.APIVersion(
'2.1')
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=False)
def test_keypair_create_with_key_type_pre_v22(self, sm_mock):
for key_type in ['x509', 'ssh']:
arglist = [
'--public-key', self.keypair.public_key,
@ -235,11 +233,8 @@ class TestKeypairCreate(TestKeypair):
'--os-compute-api-version 2.2 or greater is required',
str(ex))
def test_key_pair_create_with_user(self):
self.app.client_manager.compute.api_version = \
api_versions.APIVersion('2.10')
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=True)
def test_key_pair_create_with_user(self, sm_mock):
arglist = [
'--user', identity_fakes.user_name,
self.keypair.name,
@ -252,20 +247,16 @@ class TestKeypairCreate(TestKeypair):
columns, data = self.cmd.take_action(parsed_args)
self.keypairs_mock.create.assert_called_with(
self.sdk_client.create_keypair.assert_called_with(
name=self.keypair.name,
public_key=None,
user_id=identity_fakes.user_id,
)
self.assertEqual({}, columns)
self.assertEqual({}, data)
def test_key_pair_create_with_user_pre_v210(self):
self.app.client_manager.compute.api_version = \
api_versions.APIVersion('2.9')
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=False)
def test_key_pair_create_with_user_pre_v210(self, sm_mock):
arglist = [
'--user', identity_fakes.user_name,
self.keypair.name,
@ -291,10 +282,6 @@ class TestKeypairDelete(TestKeypair):
def setUp(self):
super(TestKeypairDelete, self).setUp()
self.keypairs_mock.get = compute_fakes.FakeKeypair.get_keypairs(
self.keypairs)
self.keypairs_mock.delete.return_value = None
self.cmd = keypair.DeleteKeypair(self.app, None)
def test_keypair_delete(self):
@ -310,7 +297,8 @@ class TestKeypairDelete(TestKeypair):
ret = self.cmd.take_action(parsed_args)
self.assertIsNone(ret)
self.keypairs_mock.delete.assert_called_with(self.keypairs[0].name)
self.sdk_client.delete_keypair.assert_called_with(
self.keypairs[0].name, ignore_missing=False)
def test_delete_multiple_keypairs(self):
arglist = []
@ -325,8 +313,8 @@ class TestKeypairDelete(TestKeypair):
calls = []
for k in self.keypairs:
calls.append(call(k.name))
self.keypairs_mock.delete.assert_has_calls(calls)
calls.append(call(k.name, ignore_missing=False))
self.sdk_client.delete_keypair.assert_has_calls(calls)
self.assertIsNone(result)
def test_delete_multiple_keypairs_with_exception(self):
@ -340,29 +328,21 @@ class TestKeypairDelete(TestKeypair):
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
find_mock_result = [self.keypairs[0], exceptions.CommandError]
with mock.patch.object(utils, 'find_resource',
side_effect=find_mock_result) as find_mock:
self.sdk_client.delete_keypair.side_effect = [
None, exceptions.CommandError]
try:
self.cmd.take_action(parsed_args)
self.fail('CommandError should be raised.')
except exceptions.CommandError as e:
self.assertEqual('1 of 2 keys failed to delete.', str(e))
find_mock.assert_any_call(
self.keypairs_mock, self.keypairs[0].name)
find_mock.assert_any_call(self.keypairs_mock, 'unexist_keypair')
self.assertEqual(2, find_mock.call_count)
self.keypairs_mock.delete.assert_called_once_with(
self.keypairs[0].name
)
def test_keypair_delete_with_user(self):
self.app.client_manager.compute.api_version = \
api_versions.APIVersion('2.10')
calls = []
for k in arglist:
calls.append(call(k, ignore_missing=False))
self.sdk_client.delete_keypair.assert_has_calls(calls)
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=True)
def test_keypair_delete_with_user(self, sm_mock):
arglist = [
'--user', identity_fakes.user_name,
self.keypairs[0].name
@ -376,12 +356,14 @@ class TestKeypairDelete(TestKeypair):
ret = self.cmd.take_action(parsed_args)
self.assertIsNone(ret)
self.keypairs_mock.delete.assert_called_with(
self.sdk_client.delete_keypair.assert_called_with(
self.keypairs[0].name,
user_id=identity_fakes.user_id,
ignore_missing=False
)
def test_keypair_delete_with_user_pre_v210(self):
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=False)
def test_keypair_delete_with_user_pre_v210(self, sm_mock):
self.app.client_manager.compute.api_version = \
api_versions.APIVersion('2.9')
@ -406,18 +388,19 @@ class TestKeypairDelete(TestKeypair):
class TestKeypairList(TestKeypair):
# Return value of self.keypairs_mock.list().
# Return value of self.sdk_client.keypairs().
keypairs = compute_fakes.FakeKeypair.create_keypairs(count=1)
def setUp(self):
super(TestKeypairList, self).setUp()
self.keypairs_mock.list.return_value = self.keypairs
self.sdk_client.keypairs.return_value = self.keypairs
# Get the command object to test
self.cmd = keypair.ListKeypair(self.app, None)
def test_keypair_list_no_options(self):
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=False)
def test_keypair_list_no_options(self, sm_mock):
arglist = []
verifylist = []
@ -430,7 +413,7 @@ class TestKeypairList(TestKeypair):
# Set expected values
self.keypairs_mock.list.assert_called_with()
self.sdk_client.keypairs.assert_called_with()
self.assertEqual(('Name', 'Fingerprint'), columns)
self.assertEqual(
@ -438,10 +421,8 @@ class TestKeypairList(TestKeypair):
tuple(data)
)
def test_keypair_list_v22(self):
self.app.client_manager.compute.api_version = \
api_versions.APIVersion('2.2')
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=True)
def test_keypair_list_v22(self, sm_mock):
arglist = []
verifylist = []
@ -454,7 +435,7 @@ class TestKeypairList(TestKeypair):
# Set expected values
self.keypairs_mock.list.assert_called_with()
self.sdk_client.keypairs.assert_called_with()
self.assertEqual(('Name', 'Fingerprint', 'Type'), columns)
self.assertEqual(
@ -466,11 +447,8 @@ class TestKeypairList(TestKeypair):
tuple(data)
)
def test_keypair_list_with_user(self):
# Filtering by user is support for nova api 2.10 or above
self.app.client_manager.compute.api_version = \
api_versions.APIVersion('2.10')
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=True)
def test_keypair_list_with_user(self, sm_mock):
users_mock = self.app.client_manager.identity.users
users_mock.reset_mock()
@ -491,7 +469,7 @@ class TestKeypairList(TestKeypair):
columns, data = self.cmd.take_action(parsed_args)
users_mock.get.assert_called_with(identity_fakes.user_name)
self.keypairs_mock.list.assert_called_with(
self.sdk_client.keypairs.assert_called_with(
user_id=identity_fakes.user_id,
)
@ -505,10 +483,8 @@ class TestKeypairList(TestKeypair):
tuple(data)
)
def test_keypair_list_with_user_pre_v210(self):
self.app.client_manager.compute.api_version = \
api_versions.APIVersion('2.9')
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=False)
def test_keypair_list_with_user_pre_v210(self, sm_mock):
arglist = [
'--user', identity_fakes.user_name,
@ -525,11 +501,8 @@ class TestKeypairList(TestKeypair):
self.assertIn(
'--os-compute-api-version 2.10 or greater is required', str(ex))
def test_keypair_list_with_project(self):
# Filtering by user is support for nova api 2.10 or above
self.app.client_manager.compute.api_version = \
api_versions.APIVersion('2.10')
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=True)
def test_keypair_list_with_project(self, sm_mock):
projects_mock = self.app.client_manager.identity.tenants
projects_mock.reset_mock()
@ -557,7 +530,7 @@ class TestKeypairList(TestKeypair):
projects_mock.get.assert_called_with(identity_fakes.project_name)
users_mock.list.assert_called_with(tenant_id=identity_fakes.project_id)
self.keypairs_mock.list.assert_called_with(
self.sdk_client.keypairs.assert_called_with(
user_id=identity_fakes.user_id,
)
@ -571,10 +544,8 @@ class TestKeypairList(TestKeypair):
tuple(data)
)
def test_keypair_list_with_project_pre_v210(self):
self.app.client_manager.compute.api_version = \
api_versions.APIVersion('2.9')
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=False)
def test_keypair_list_with_project_pre_v210(self, sm_mock):
arglist = ['--project', identity_fakes.project_name]
verifylist = [('project', identity_fakes.project_name)]
@ -589,10 +560,6 @@ class TestKeypairList(TestKeypair):
def test_keypair_list_conflicting_user_options(self):
# Filtering by user is support for nova api 2.10 or above
self.app.client_manager.compute.api_version = \
api_versions.APIVersion('2.10')
arglist = [
'--user', identity_fakes.user_name,
'--project', identity_fakes.project_name,
@ -610,7 +577,7 @@ class TestKeypairShow(TestKeypair):
def setUp(self):
super(TestKeypairShow, self).setUp()
self.keypairs_mock.get.return_value = self.keypair
self.sdk_client.find_keypair.return_value = self.keypair
self.cmd = keypair.ShowKeypair(self.app, None)
@ -641,7 +608,7 @@ class TestKeypairShow(TestKeypair):
# overwrite the setup one because we want to omit private_key
self.keypair = compute_fakes.FakeKeypair.create_one_keypair(
no_pri=True)
self.keypairs_mock.get.return_value = self.keypair
self.sdk_client.find_keypair.return_value = self.keypair
self.data = (
self.keypair.fingerprint,
@ -660,8 +627,9 @@ class TestKeypairShow(TestKeypair):
columns, data = self.cmd.take_action(parsed_args)
self.keypairs_mock.get.assert_called_with(
self.sdk_client.find_keypair.assert_called_with(
self.keypair.name,
ignore_missing=False
)
self.assertEqual(self.columns, columns)
@ -685,12 +653,13 @@ class TestKeypairShow(TestKeypair):
self.assertEqual({}, columns)
self.assertEqual({}, data)
def test_keypair_show_with_user(self):
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=True)
def test_keypair_show_with_user(self, sm_mock):
# overwrite the setup one because we want to omit private_key
self.keypair = compute_fakes.FakeKeypair.create_one_keypair(
no_pri=True)
self.keypairs_mock.get.return_value = self.keypair
self.sdk_client.find_keypair.return_value = self.keypair
self.data = (
self.keypair.fingerprint,
@ -699,9 +668,6 @@ class TestKeypairShow(TestKeypair):
self.keypair.user_id
)
self.app.client_manager.compute.api_version = \
api_versions.APIVersion('2.10')
arglist = [
'--user', identity_fakes.user_name,
self.keypair.name,
@ -715,14 +681,17 @@ class TestKeypairShow(TestKeypair):
columns, data = self.cmd.take_action(parsed_args)
self.users_mock.get.assert_called_with(identity_fakes.user_name)
self.keypairs_mock.get.assert_called_with(
self.sdk_client.find_keypair.assert_called_with(
self.keypair.name,
ignore_missing=False,
user_id=identity_fakes.user_id
)
self.assertEqual(self.columns, columns)
self.assertEqual(self.data, data)
def test_keypair_show_with_user_pre_v210(self):
@mock.patch.object(sdk_utils, 'supports_microversion', return_value=False)
def test_keypair_show_with_user_pre_v210(self, sm_mock):
arglist = [
'--user', identity_fakes.user_name,

3
releasenotes/notes/switch-keypair-to-sdk-81e28380e66a7f9c.yaml Normal file
View File

@ -0,0 +1,3 @@
---
Features:
- Switch 'openstack keypair' commands to use OpenStackSDK