diff --git a/openstackclient/compute/v2/keypair.py b/openstackclient/compute/v2/keypair.py index 726dbfb9aa..19e30bff10 100644 --- a/openstackclient/compute/v2/keypair.py +++ b/openstackclient/compute/v2/keypair.py @@ -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 ({}, {}) diff --git a/openstackclient/tests/unit/compute/v2/test_keypair.py b/openstackclient/tests/unit/compute/v2/test_keypair.py index 9632a6671a..5a17808fa1 100644 --- a/openstackclient/tests/unit/compute/v2/test_keypair.py +++ b/openstackclient/tests/unit/compute/v2/test_keypair.py @@ -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: - 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)) + 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, diff --git a/releasenotes/notes/switch-keypair-to-sdk-81e28380e66a7f9c.yaml b/releasenotes/notes/switch-keypair-to-sdk-81e28380e66a7f9c.yaml new file mode 100644 index 0000000000..17e53c4cd6 --- /dev/null +++ b/releasenotes/notes/switch-keypair-to-sdk-81e28380e66a7f9c.yaml @@ -0,0 +1,3 @@ +--- +Features: + - Switch 'openstack keypair' commands to use OpenStackSDK