Merge "Add owner validation for "openstack image create/set""

Author: Jenkins

doc/source/command-objects/image.rst

@@ -33,6 +33,7 @@ Create/upload an image
         [--public | --private]
         [--property <key=value> [...] ]
         [--tag <tag> [...] ]
+        [--project-domain <project-domain>]
         <image-name>
 
 .. option:: --id <id>
@@ -127,6 +128,11 @@ Create/upload an image
 
     .. versionadded:: 2
 
+.. option:: --project-domain <project-domain>
+
+    Domain the project belongs to (name or ID).
+    This can be used in case collisions between project names exist.
+
 .. describe:: <image-name>
 
     New image name
@@ -244,6 +250,7 @@ Set image properties
         [--os-version <os-version>]
         [--ramdisk-id <ramdisk-id>]
         [--activate|--deactivate]
+        [--project-domain <project-domain>]
         <image>
 
 .. option:: --name <name>
@@ -400,6 +407,11 @@ Set image properties
 
     .. versionadded:: 2
 
+.. option:: --project-domain <project-domain>
+
+    Domain the project belongs to (name or ID).
+    This can be used in case collisions between project names exist.
+
 .. describe:: <image>
 
     Image to modify (name or ID)

openstackclient/image/v2/image.py

@@ -220,6 +220,7 @@ def get_parser(self, prog_name):
             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 @@ def get_parser(self, prog_name):
 
     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 @@ def take_action(self, parsed_args):
             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 @@ def get_parser(self, prog_name):
             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 @@ def get_parser(self, prog_name):
 
     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 @@ def take_action(self, parsed_args):
             # 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:

openstackclient/tests/image/v2/test_image.py

@@ -57,6 +57,19 @@ def setUp(self):
 
         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 @@ def test_image_reserve_options(self, mock_open):
                 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 @@ def test_image_reserve_options(self, mock_open):
             ('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 @@ def test_image_reserve_options(self, mock_open):
             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 @@ def test_image_reserve_options(self, mock_open):
             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 @@ def setUp(self):
             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 @@ def setUp(self):
     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 @@ def test_image_set_options(self):
 
         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 @@ def test_image_set_options(self):
         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',