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
This commit is contained in:
Alan Bishop 2018-08-10 10:20:34 -04:00
parent a051bda111
commit 030fd71390
3 changed files with 41 additions and 98 deletions

View File

@ -126,8 +126,6 @@ class TestVolumeCreate(TestVolume):
name=self.new_volume.name, name=self.new_volume.name,
description=None, description=None,
volume_type=None, volume_type=None,
user_id=None,
project_id=None,
availability_zone=None, availability_zone=None,
metadata=None, metadata=None,
imageRef=None, imageRef=None,
@ -177,8 +175,6 @@ class TestVolumeCreate(TestVolume):
name=self.new_volume.name, name=self.new_volume.name,
description=self.new_volume.description, description=self.new_volume.description,
volume_type=self.new_volume.volume_type, volume_type=self.new_volume.volume_type,
user_id=None,
project_id=None,
availability_zone=self.new_volume.availability_zone, availability_zone=self.new_volume.availability_zone,
metadata=None, metadata=None,
imageRef=None, imageRef=None,
@ -191,95 +187,39 @@ class TestVolumeCreate(TestVolume):
self.assertEqual(self.columns, columns) self.assertEqual(self.columns, columns)
self.assertEqual(self.datalist, data) self.assertEqual(self.datalist, data)
def test_volume_create_user_project_id(self): def test_volume_create_user(self):
# Return a project
self.projects_mock.get.return_value = self.project
# Return a user
self.users_mock.get.return_value = self.user
arglist = [ arglist = [
'--size', str(self.new_volume.size), '--size', str(self.new_volume.size),
'--project', self.project.id,
'--user', self.user.id, '--user', self.user.id,
self.new_volume.name, self.new_volume.name,
] ]
verifylist = [ verifylist = [
('size', self.new_volume.size), ('size', self.new_volume.size),
('project', self.project.id),
('user', self.user.id), ('user', self.user.id),
('name', self.new_volume.name), ('name', self.new_volume.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
# In base command class ShowOne in cliff, abstract method take_action() self.assertRaises(exceptions.CommandError, self.cmd.take_action,
# returns a two-part tuple with a tuple of column names and a tuple of parsed_args)
# data to be shown. self.volumes_mock.create.assert_not_called()
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
def test_volume_create_project(self):
arglist = [ arglist = [
'--size', str(self.new_volume.size), '--size', str(self.new_volume.size),
'--project', self.project.name, '--project', self.project.id,
'--user', self.user.name,
self.new_volume.name, self.new_volume.name,
] ]
verifylist = [ verifylist = [
('size', self.new_volume.size), ('size', self.new_volume.size),
('project', self.project.name), ('project', self.project.id),
('user', self.user.name),
('name', self.new_volume.name), ('name', self.new_volume.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
# In base command class ShowOne in cliff, abstract method take_action() self.assertRaises(exceptions.CommandError, self.cmd.take_action,
# returns a two-part tuple with a tuple of column names and a tuple of parsed_args)
# data to be shown. self.volumes_mock.create.assert_not_called()
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_properties(self): def test_volume_create_properties(self):
arglist = [ arglist = [
@ -306,8 +246,6 @@ class TestVolumeCreate(TestVolume):
name=self.new_volume.name, name=self.new_volume.name,
description=None, description=None,
volume_type=None, volume_type=None,
user_id=None,
project_id=None,
availability_zone=None, availability_zone=None,
metadata={'Alpha': 'a', 'Beta': 'b'}, metadata={'Alpha': 'a', 'Beta': 'b'},
imageRef=None, imageRef=None,
@ -347,8 +285,6 @@ class TestVolumeCreate(TestVolume):
name=self.new_volume.name, name=self.new_volume.name,
description=None, description=None,
volume_type=None, volume_type=None,
user_id=None,
project_id=None,
availability_zone=None, availability_zone=None,
metadata=None, metadata=None,
imageRef=image.id, imageRef=image.id,
@ -388,8 +324,6 @@ class TestVolumeCreate(TestVolume):
name=self.new_volume.name, name=self.new_volume.name,
description=None, description=None,
volume_type=None, volume_type=None,
user_id=None,
project_id=None,
availability_zone=None, availability_zone=None,
metadata=None, metadata=None,
imageRef=image.id, imageRef=image.id,
@ -428,8 +362,6 @@ class TestVolumeCreate(TestVolume):
name=self.new_volume.name, name=self.new_volume.name,
description=None, description=None,
volume_type=None, volume_type=None,
user_id=None,
project_id=None,
availability_zone=None, availability_zone=None,
metadata=None, metadata=None,
imageRef=None, imageRef=None,
@ -469,8 +401,6 @@ class TestVolumeCreate(TestVolume):
name=self.new_volume.name, name=self.new_volume.name,
description=None, description=None,
volume_type=None, volume_type=None,
user_id=None,
project_id=None,
availability_zone=None, availability_zone=None,
metadata=None, metadata=None,
imageRef=None, imageRef=None,
@ -514,8 +444,6 @@ class TestVolumeCreate(TestVolume):
name=self.new_volume.name, name=self.new_volume.name,
description=None, description=None,
volume_type=None, volume_type=None,
user_id=None,
project_id=None,
availability_zone=None, availability_zone=None,
metadata=None, metadata=None,
imageRef=None, imageRef=None,
@ -568,8 +496,6 @@ class TestVolumeCreate(TestVolume):
name=self.new_volume.name, name=self.new_volume.name,
description=None, description=None,
volume_type=None, volume_type=None,
user_id=None,
project_id=None,
availability_zone=None, availability_zone=None,
metadata=None, metadata=None,
imageRef=None, imageRef=None,

View File

@ -96,12 +96,12 @@ class CreateVolume(command.ShowOne):
parser.add_argument( parser.add_argument(
'--user', '--user',
metavar='<user>', metavar='<user>',
help=_('Specify an alternate user (name or ID)'), help=argparse.SUPPRESS,
) )
parser.add_argument( parser.add_argument(
'--project', '--project',
metavar='<project>', metavar='<project>',
help=_('Specify an alternate project (name or ID)'), help=argparse.SUPPRESS,
) )
parser.add_argument( parser.add_argument(
"--availability-zone", "--availability-zone",
@ -159,7 +159,6 @@ class CreateVolume(command.ShowOne):
def take_action(self, parsed_args): def take_action(self, parsed_args):
_check_size_arg(parsed_args) _check_size_arg(parsed_args)
identity_client = self.app.client_manager.identity
volume_client = self.app.client_manager.volume volume_client = self.app.client_manager.volume
image_client = self.app.client_manager.image image_client = self.app.client_manager.image
@ -197,17 +196,22 @@ class CreateVolume(command.ShowOne):
# snapshot size. # snapshot size.
size = max(size or 0, snapshot_obj.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: if parsed_args.project:
project = utils.find_resource( raise exceptions.CommandError(
identity_client.projects, _("ERROR: --project is deprecated, please use"
parsed_args.project).id " --os-project-name or --os-project-id instead."))
user = None
if parsed_args.user: if parsed_args.user:
user = utils.find_resource( raise exceptions.CommandError(
identity_client.users, _("ERROR: --user is deprecated, please use"
parsed_args.user).id " --os-username instead."))
volume = volume_client.volumes.create( volume = volume_client.volumes.create(
size=size, size=size,
@ -215,8 +219,6 @@ class CreateVolume(command.ShowOne):
name=parsed_args.name, name=parsed_args.name,
description=parsed_args.description, description=parsed_args.description,
volume_type=parsed_args.type, volume_type=parsed_args.type,
user_id=user,
project_id=project,
availability_zone=parsed_args.availability_zone, availability_zone=parsed_args.availability_zone,
metadata=parsed_args.property, metadata=parsed_args.property,
imageRef=image, imageRef=image,

View File

@ -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).