From 0a444fc949584c9ac2a555bfa9ad221913ad4779 Mon Sep 17 00:00:00 2001 From: xiexs Date: Sat, 5 Dec 2015 19:25:30 +0800 Subject: [PATCH] Add owner validation for "openstack image create/set" Owner validation is necessary if a new image owner will be created/set. Change-Id: I621774e02866bfa98a31b613deff5d7b6a962737 Closes-Bug: #1517134 --- doc/source/command-objects/image.rst | 12 +++ openstackclient/image/v2/image.py | 18 ++++ openstackclient/tests/image/v2/test_image.py | 90 +++++++++++++++++++- 3 files changed, 116 insertions(+), 4 deletions(-) diff --git a/doc/source/command-objects/image.rst b/doc/source/command-objects/image.rst index 6a4782ea5e..f1893efaec 100644 --- a/doc/source/command-objects/image.rst +++ b/doc/source/command-objects/image.rst @@ -33,6 +33,7 @@ Create/upload an image [--public | --private] [--property [...] ] [--tag [...] ] + [--project-domain ] .. option:: --id @@ -127,6 +128,11 @@ Create/upload an image .. versionadded:: 2 +.. option:: --project-domain + + Domain the project belongs to (name or ID). + This can be used in case collisions between project names exist. + .. describe:: New image name @@ -244,6 +250,7 @@ Set image properties [--os-version ] [--ramdisk-id ] [--activate|--deactivate] + [--project-domain ] .. option:: --name @@ -400,6 +407,11 @@ Set image properties .. versionadded:: 2 +.. option:: --project-domain + + Domain the project belongs to (name or ID). + This can be used in case collisions between project names exist. + .. describe:: Image to modify (name or ID) diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py index 1fcb92d9a5..ad536ba26b 100644 --- a/openstackclient/image/v2/image.py +++ b/openstackclient/image/v2/image.py @@ -220,6 +220,7 @@ class CreateImage(show.ShowOne): help="Set a tag on this image " "(repeat option to set multiple tags)", ) + common.add_project_domain_option_to_parser(parser) for deadopt in self.deadopts: parser.add_argument( "--%s" % deadopt, @@ -231,6 +232,7 @@ class CreateImage(show.ShowOne): def take_action(self, parsed_args): self.log.debug("take_action(%s)", parsed_args) + identity_client = self.app.client_manager.identity image_client = self.app.client_manager.image for deadopt in self.deadopts: @@ -285,6 +287,13 @@ class CreateImage(show.ShowOne): self.log.warning("Failed to get an image file.") return {}, {} + if parsed_args.owner: + kwargs['owner'] = common.find_project( + identity_client, + parsed_args.owner, + parsed_args.project_domain, + ).id + # If a volume is specified. if parsed_args.volume: volume_client = self.app.client_manager.volume @@ -704,6 +713,7 @@ class SetImage(command.Command): action="store_true", help="Activate the image", ) + common.add_project_domain_option_to_parser(parser) for deadopt in self.deadopts: parser.add_argument( "--%s" % deadopt, @@ -715,6 +725,7 @@ class SetImage(command.Command): def take_action(self, parsed_args): self.log.debug("take_action(%s)", parsed_args) + identity_client = self.app.client_manager.identity image_client = self.app.client_manager.image for deadopt in self.deadopts: @@ -779,6 +790,13 @@ class SetImage(command.Command): # Tags should be extended, but duplicates removed kwargs['tags'] = list(set(image.tags).union(set(parsed_args.tags))) + if parsed_args.owner: + kwargs['owner'] = common.find_project( + identity_client, + parsed_args.owner, + parsed_args.project_domain, + ).id + try: image = image_client.images.update(image.id, **kwargs) except Exception as e: diff --git a/openstackclient/tests/image/v2/test_image.py b/openstackclient/tests/image/v2/test_image.py index 118a119fa7..0218241397 100644 --- a/openstackclient/tests/image/v2/test_image.py +++ b/openstackclient/tests/image/v2/test_image.py @@ -57,6 +57,19 @@ class TestImageCreate(TestImage): self.new_image = image_fakes.FakeImage.create_one_image() self.images_mock.create.return_value = self.new_image + + self.project_mock.get.return_value = fakes.FakeResource( + None, + copy.deepcopy(identity_fakes.PROJECT), + loaded=True, + ) + + self.domain_mock.get.return_value = fakes.FakeResource( + None, + copy.deepcopy(identity_fakes.DOMAIN), + loaded=True, + ) + # This is the return value for utils.find_resource() self.images_mock.get.return_value = copy.deepcopy( image_fakes.FakeImage.get_image_info(self.new_image)) @@ -123,6 +136,7 @@ class TestImageCreate(TestImage): if self.new_image.protected else '--unprotected'), ('--private' if self.new_image.visibility == 'private' else '--public'), + '--project-domain', identity_fakes.domain_id, self.new_image.name, ] verifylist = [ @@ -135,6 +149,7 @@ class TestImageCreate(TestImage): ('unprotected', not self.new_image.protected), ('public', self.new_image.visibility == 'public'), ('private', self.new_image.visibility == 'private'), + ('project_domain', identity_fakes.domain_id), ('name', self.new_image.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -149,7 +164,7 @@ class TestImageCreate(TestImage): disk_format='fs', min_disk=10, min_ram=4, - owner=self.new_image.owner, + owner=identity_fakes.project_id, protected=self.new_image.protected, visibility=self.new_image.visibility, ) @@ -168,6 +183,40 @@ class TestImageCreate(TestImage): image_fakes.FakeImage.get_image_data(self.new_image), data) + def test_image_create_with_unexist_owner(self): + self.project_mock.get.side_effect = exceptions.NotFound(None) + self.project_mock.find.side_effect = exceptions.NotFound(None) + + arglist = [ + '--container-format', 'ovf', + '--disk-format', 'fs', + '--min-disk', '10', + '--min-ram', '4', + '--owner', 'unexist_owner', + '--protected', + '--private', + image_fakes.image_name, + ] + verifylist = [ + ('container_format', 'ovf'), + ('disk_format', 'fs'), + ('min_disk', 10), + ('min_ram', 4), + ('owner', 'unexist_owner'), + ('protected', True), + ('unprotected', False), + ('public', False), + ('private', True), + ('name', image_fakes.image_name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaises( + exceptions.CommandError, + self.cmd.take_action, + parsed_args, + ) + @mock.patch('glanceclient.common.utils.get_data_file', name='Open') def test_image_create_file(self, mock_open): mock_file = mock.MagicMock(name='File') @@ -686,6 +735,18 @@ class TestImageSet(TestImage): schemas.SchemaBasedModel, ) + self.project_mock.get.return_value = fakes.FakeResource( + None, + copy.deepcopy(identity_fakes.PROJECT), + loaded=True, + ) + + self.domain_mock.get.return_value = fakes.FakeResource( + None, + copy.deepcopy(identity_fakes.DOMAIN), + loaded=True, + ) + self.images_mock.get.return_value = self.model(**image_fakes.IMAGE) self.images_mock.update.return_value = self.model(**image_fakes.IMAGE) # Get the command object to test @@ -694,20 +755,22 @@ class TestImageSet(TestImage): def test_image_set_options(self): arglist = [ '--name', 'new-name', - '--owner', 'new-owner', + '--owner', identity_fakes.project_name, '--min-disk', '2', '--min-ram', '4', '--container-format', 'ovf', '--disk-format', 'vmdk', + '--project-domain', identity_fakes.domain_id, image_fakes.image_id, ] verifylist = [ ('name', 'new-name'), - ('owner', 'new-owner'), + ('owner', identity_fakes.project_name), ('min_disk', 2), ('min_ram', 4), ('container_format', 'ovf'), ('disk_format', 'vmdk'), + ('project_domain', identity_fakes.domain_id), ('image', image_fakes.image_id), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -717,7 +780,7 @@ class TestImageSet(TestImage): kwargs = { 'name': 'new-name', - 'owner': 'new-owner', + 'owner': identity_fakes.project_id, 'min_disk': 2, 'min_ram': 4, 'container_format': 'ovf', @@ -727,6 +790,25 @@ class TestImageSet(TestImage): self.images_mock.update.assert_called_with( image_fakes.image_id, **kwargs) + def test_image_set_with_unexist_owner(self): + self.project_mock.get.side_effect = exceptions.NotFound(None) + self.project_mock.find.side_effect = exceptions.NotFound(None) + + arglist = [ + '--owner', 'unexist_owner', + image_fakes.image_id, + ] + verifylist = [ + ('owner', 'unexist_owner'), + ('image', image_fakes.image_id), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaises( + exceptions.CommandError, + self.cmd.take_action, parsed_args) + def test_image_set_bools1(self): arglist = [ '--protected',