From 030fd71390e8e4b8413e916e64076337035f9aa7 Mon Sep 17 00:00:00 2001 From: Alan Bishop Date: Fri, 10 Aug 2018 10:20:34 -0400 Subject: [PATCH] Deprecate volume create --project and --user options Cinder's volume create API does not support overriding the project_id and user_id, and it silently igores those API inputs. Cinder always uses the project and user info in the keystone identity associated with the API request. If a user specifies the --project or --user option, the volume create is aborted and a CommandError exception is raised. This prevents a volume from being created, but without the desired project/user values. A user wishing to specify alternate values can still do so using identity overrides (e.g. --os-username, --os-project-id). Story: 2002583 Task: 22192 Change-Id: Ia9f910ea1b0e61797e8c8c463fa28e7390f15bf9 --- .../tests/unit/volume/v2/test_volume.py | 94 ++----------------- openstackclient/volume/v2/volume.py | 30 +++--- .../notes/bug-1777153-750e6044aa28d5d8.yaml | 15 +++ 3 files changed, 41 insertions(+), 98 deletions(-) create mode 100644 releasenotes/notes/bug-1777153-750e6044aa28d5d8.yaml diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py index 971567cb85..bb6263bbe2 100644 --- a/openstackclient/tests/unit/volume/v2/test_volume.py +++ b/openstackclient/tests/unit/volume/v2/test_volume.py @@ -126,8 +126,6 @@ class TestVolumeCreate(TestVolume): name=self.new_volume.name, description=None, volume_type=None, - user_id=None, - project_id=None, availability_zone=None, metadata=None, imageRef=None, @@ -177,8 +175,6 @@ class TestVolumeCreate(TestVolume): name=self.new_volume.name, description=self.new_volume.description, volume_type=self.new_volume.volume_type, - user_id=None, - project_id=None, availability_zone=self.new_volume.availability_zone, metadata=None, imageRef=None, @@ -191,95 +187,39 @@ class TestVolumeCreate(TestVolume): self.assertEqual(self.columns, columns) self.assertEqual(self.datalist, data) - def test_volume_create_user_project_id(self): - # Return a project - self.projects_mock.get.return_value = self.project - # Return a user - self.users_mock.get.return_value = self.user - + def test_volume_create_user(self): arglist = [ '--size', str(self.new_volume.size), - '--project', self.project.id, '--user', self.user.id, self.new_volume.name, ] verifylist = [ ('size', self.new_volume.size), - ('project', self.project.id), ('user', self.user.id), ('name', self.new_volume.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - # In base command class ShowOne in cliff, abstract method take_action() - # returns a two-part tuple with a tuple of column names and a tuple of - # data to be shown. - columns, data = self.cmd.take_action(parsed_args) - - self.volumes_mock.create.assert_called_with( - size=self.new_volume.size, - snapshot_id=None, - name=self.new_volume.name, - description=None, - volume_type=None, - user_id=self.user.id, - project_id=self.project.id, - availability_zone=None, - metadata=None, - imageRef=None, - source_volid=None, - consistencygroup_id=None, - multiattach=False, - scheduler_hints=None, - ) - - self.assertEqual(self.columns, columns) - self.assertEqual(self.datalist, data) - - def test_volume_create_user_project_name(self): - # Return a project - self.projects_mock.get.return_value = self.project - # Return a user - self.users_mock.get.return_value = self.user + self.assertRaises(exceptions.CommandError, self.cmd.take_action, + parsed_args) + self.volumes_mock.create.assert_not_called() + def test_volume_create_project(self): arglist = [ '--size', str(self.new_volume.size), - '--project', self.project.name, - '--user', self.user.name, + '--project', self.project.id, self.new_volume.name, ] verifylist = [ ('size', self.new_volume.size), - ('project', self.project.name), - ('user', self.user.name), + ('project', self.project.id), ('name', self.new_volume.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - # In base command class ShowOne in cliff, abstract method take_action() - # returns a two-part tuple with a tuple of column names and a tuple of - # data to be shown. - columns, data = self.cmd.take_action(parsed_args) - - self.volumes_mock.create.assert_called_with( - size=self.new_volume.size, - snapshot_id=None, - name=self.new_volume.name, - description=None, - volume_type=None, - user_id=self.user.id, - project_id=self.project.id, - availability_zone=None, - metadata=None, - imageRef=None, - source_volid=None, - consistencygroup_id=None, - multiattach=False, - scheduler_hints=None, - ) - - self.assertEqual(self.columns, columns) - self.assertEqual(self.datalist, data) + self.assertRaises(exceptions.CommandError, self.cmd.take_action, + parsed_args) + self.volumes_mock.create.assert_not_called() def test_volume_create_properties(self): arglist = [ @@ -306,8 +246,6 @@ class TestVolumeCreate(TestVolume): name=self.new_volume.name, description=None, volume_type=None, - user_id=None, - project_id=None, availability_zone=None, metadata={'Alpha': 'a', 'Beta': 'b'}, imageRef=None, @@ -347,8 +285,6 @@ class TestVolumeCreate(TestVolume): name=self.new_volume.name, description=None, volume_type=None, - user_id=None, - project_id=None, availability_zone=None, metadata=None, imageRef=image.id, @@ -388,8 +324,6 @@ class TestVolumeCreate(TestVolume): name=self.new_volume.name, description=None, volume_type=None, - user_id=None, - project_id=None, availability_zone=None, metadata=None, imageRef=image.id, @@ -428,8 +362,6 @@ class TestVolumeCreate(TestVolume): name=self.new_volume.name, description=None, volume_type=None, - user_id=None, - project_id=None, availability_zone=None, metadata=None, imageRef=None, @@ -469,8 +401,6 @@ class TestVolumeCreate(TestVolume): name=self.new_volume.name, description=None, volume_type=None, - user_id=None, - project_id=None, availability_zone=None, metadata=None, imageRef=None, @@ -514,8 +444,6 @@ class TestVolumeCreate(TestVolume): name=self.new_volume.name, description=None, volume_type=None, - user_id=None, - project_id=None, availability_zone=None, metadata=None, imageRef=None, @@ -568,8 +496,6 @@ class TestVolumeCreate(TestVolume): name=self.new_volume.name, description=None, volume_type=None, - user_id=None, - project_id=None, availability_zone=None, metadata=None, imageRef=None, diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py index fc648cef39..8ab61d2aa1 100644 --- a/openstackclient/volume/v2/volume.py +++ b/openstackclient/volume/v2/volume.py @@ -96,12 +96,12 @@ class CreateVolume(command.ShowOne): parser.add_argument( '--user', metavar='', - help=_('Specify an alternate user (name or ID)'), + help=argparse.SUPPRESS, ) parser.add_argument( '--project', metavar='', - help=_('Specify an alternate project (name or ID)'), + help=argparse.SUPPRESS, ) parser.add_argument( "--availability-zone", @@ -159,7 +159,6 @@ class CreateVolume(command.ShowOne): def take_action(self, parsed_args): _check_size_arg(parsed_args) - identity_client = self.app.client_manager.identity volume_client = self.app.client_manager.volume image_client = self.app.client_manager.image @@ -197,17 +196,22 @@ class CreateVolume(command.ShowOne): # snapshot size. size = max(size or 0, snapshot_obj.size) - project = None + # NOTE(abishop): Cinder's volumes.create() has 'project_id' and + # 'user_id' args, but they're not wired up to anything. The only way + # to specify an alternate project or user for the volume is to use + # the identity overrides (e.g. "--os-project-id"). + # + # Now, if the project or user arg is specified then the command is + # rejected. Otherwise, Cinder would actually create a volume, but + # without the specified property. if parsed_args.project: - project = utils.find_resource( - identity_client.projects, - parsed_args.project).id - - user = None + raise exceptions.CommandError( + _("ERROR: --project is deprecated, please use" + " --os-project-name or --os-project-id instead.")) if parsed_args.user: - user = utils.find_resource( - identity_client.users, - parsed_args.user).id + raise exceptions.CommandError( + _("ERROR: --user is deprecated, please use" + " --os-username instead.")) volume = volume_client.volumes.create( size=size, @@ -215,8 +219,6 @@ class CreateVolume(command.ShowOne): name=parsed_args.name, description=parsed_args.description, volume_type=parsed_args.type, - user_id=user, - project_id=project, availability_zone=parsed_args.availability_zone, metadata=parsed_args.property, imageRef=image, diff --git a/releasenotes/notes/bug-1777153-750e6044aa28d5d8.yaml b/releasenotes/notes/bug-1777153-750e6044aa28d5d8.yaml new file mode 100644 index 0000000000..a2a9b51aea --- /dev/null +++ b/releasenotes/notes/bug-1777153-750e6044aa28d5d8.yaml @@ -0,0 +1,15 @@ +--- +deprecations: + - | + The ``--project`` and ``--user`` options for the ``volume create`` + command have been deprecated. They are deprecated because Cinder's + volume create API ignores the corresponding API inputs. +fixes: + - | + Fix ``volume create`` by removing two broken options. The ``--project`` + and ``--user`` options were intended to specify an alternate project + and/or user for the volume, but the Volume service's API does not + support this behavior. This caused the volume to be created, but + without the expected project/user values. However, an alternate + project and/or user may be specified using identity overrides (e.g. + --os-username, --os-project-id).