Add rc check for POST Device Profile

At present, when creating a device profile, the function
_validate_post_request only checks if the keys of groups
matches that in ["resources:", "trait:", "accel:"] without
checking the string following "trait:","resources:",this
introduces risks.

For example, the string followed by "trait:" default should be
"CUSTOM_", what follows "resources:" should be a valid resource type,
but the fact is that users may make typos such as adding extra spaces
like "resources:  FPGA", or other format errors. This kind of error
cannot be detected by the current _validate_post_request, and it won't
be exposed until nova parses the rc of cyborg.

This is not in compliance with the process specification and potentially
introduces greater risks. Therefore, it is necessary to implement a more
strict inspection of rc in device profile POST API.

This patch added the rc check.
1.Check the legality of rc name
2.Space removal in rc
3.rc value check

Change-Id: Iabad6b1c3ad2b4f61e69249a2867bd68107d47d5
Story: 2008143
Task: 40883

cyborg/api/controllers/v2/device_profiles.py

@@ -28,6 +28,7 @@ from cyborg.api.controllers import link
 from cyborg.api.controllers import types
 from cyborg.api import expose
 from cyborg.common import authorize_wsgi
+from cyborg.common import constants
 from cyborg.common import exception
 from cyborg import objects
 LOG = log.getLogger(__name__)
@@ -160,6 +161,24 @@ class DeviceProfilesController(base.CyborgController,
                         del group[key]
                         standard_key = "trait:" + inner_trait
                         group[standard_key] = value
+                # check rc name and it's value
+                if key.startswith("resources:"):
+                    inner_origin_rc = ":".join(key.split(":")[1:])
+                    inner_rc = inner_origin_rc.strip(" ")
+                    if inner_rc not in constants.SUPPORT_RESOURCES and \
+                        not inner_rc.startswith('CUSTOM_'):
+                        raise exception.InvalidParameterValue(
+                            err="Unsupported resource class %s" % inner_rc)
+                    try:
+                        int(value)
+                    except ValueError:
+                        raise exception.InvalidParameterValue(
+                            err="Resources nummber %s is invalid" % value)
+                    # strip " " and update old group key.
+                    if inner_origin_rc != inner_rc:
+                        del group[key]
+                        standard_key = "resources:" + inner_rc
+                        group[standard_key] = value
 
     def _get_device_profile_list(self, names=None, uuid=None):
         """Get a list of API objects representing device profiles."""

cyborg/objects/extarq/ext_arq_job.py

@@ -49,7 +49,7 @@ class ExtARQJobMixin(object):
     def get_suitable_ext_arq(cls, context, uuid):
         """From the inherit subclass find the suitable ExtARQ."""
         extarq = cls.get(context, uuid)
-        typ, _ = extarq.get_resources_from_device_profile_group()
+        typ = extarq.get_resources_from_device_profile_group()
         factory = cls.factory(typ)
         if factory != cls:
             return factory.get(context, uuid)
@@ -222,19 +222,8 @@ class ExtARQJobMixin(object):
         if not resources:
             raise exception.InvalidParameterValue(
                 'No resources in device_profile_group: %s' % group)
-        res_type, res_num = resources[0]
-        # TODO(Sundar): this should be caught in ARQ create, not bind.
-        if res_type not in constants.SUPPORT_RESOURCES:
-            raise exception.InvalidParameterValue(
-                'Unsupport resources %s from device_profile_group: %s' %
-                (res_type, group))
-        try:
-            res_num = int(res_num)
-        except ValueError:
-            raise exception.InvalidParameterValue(
-                'Resources nummber is a invalid in'
-                ' device_profile_group: %s' % group)
-        return res_type, res_num
+        res_type = resources[0][0]
+        return res_type
 
     @classmethod
     def apply_patch(cls, context, patch_list, valid_fields):

cyborg/tests/unit/api/controllers/v2/test_device_profiles.py

@@ -124,6 +124,57 @@ class TestDeviceProfileController(v2_test.APITestV2):
         # {'trait:CUSTOM_FPGA_INTEL_PAC_ARRIA10': 'required}
         self.assertTrue(out_dp['groups'] == self.fake_dp_objs[0]['groups'])
 
+    @mock.patch('cyborg.conductor.rpcapi.ConductorAPI.device_profile_create')
+    def test_create_with_extra_space_in_rc(self, mock_cond_dp):
+        test_unsupport_dp = self.fake_dps[0]
+
+        # generate a requested dp which has extra space in rc
+        del test_unsupport_dp['groups'][0]['resources:FPGA']
+        test_unsupport_dp['groups'][0]['resources: FPGA '] = '1'
+        dp = [test_unsupport_dp]
+
+        mock_cond_dp.return_value = self.fake_dp_objs[0]
+        dp[0]['created_at'] = str(dp[0]['created_at'])
+
+        response = self.post_json(self.DP_URL, dp, headers=self.headers)
+        out_dp = jsonutils.loads(response.controller_output)
+
+        # check that the extra space in rc:{'resources: FPGA ': '1'} is
+        # successful stripped by the _validate_post_request function, and
+        # the created device_profile has no extra space in
+        # rc:{'resources:FPGA': '1'}
+        self.assertTrue(out_dp['groups'] == self.fake_dp_objs[0]['groups'])
+
+    def test_create_with_unsupported_rc(self):
+        test_unsupport_dp = self.fake_dps[0]
+        # generate a special rc for test
+        del test_unsupport_dp['groups'][0]['resources:FPGA']
+        test_unsupport_dp['groups'][0]["resources:FAKE_RC"] = '1'
+
+        dp = [test_unsupport_dp]
+        dp[0]['created_at'] = str(dp[0]['created_at'])
+        self.assertRaisesRegex(
+            webtest.app.AppError,
+            ".*Unsupported resource class FAKE_RC.*",
+            self.post_json,
+            self.DP_URL,
+            dp,
+            headers=self.headers)
+
+    def test_create_with_invalid_resource_value(self):
+        test_unsupport_dp = self.fake_dps[0]
+        del test_unsupport_dp['groups'][0]['resources:FPGA']
+        test_unsupport_dp['groups'][0]["resources:CUSTOM_FAKE_RC"] = 'fake'
+        dp = [test_unsupport_dp]
+        dp[0]['created_at'] = str(dp[0]['created_at'])
+        self.assertRaisesRegex(
+            webtest.app.AppError,
+            ".*Resources nummber fake is invalid.*",
+            self.post_json,
+            self.DP_URL,
+            dp,
+            headers=self.headers)
+
     @mock.patch('cyborg.conductor.rpcapi.ConductorAPI.device_profile_delete')
     @mock.patch('cyborg.objects.DeviceProfile.get_by_name')
     @mock.patch('cyborg.objects.DeviceProfile.get_by_uuid')

cyborg/tests/unit/fake_device_profile.py

@@ -41,7 +41,7 @@ def _get_device_profiles_as_dict():
         "created_at": date1,
         "updated_at": None,
         "groups": [
-            {"resources:ACCELERATOR_FPGA": "1",
+            {"resources:FPGA": "1",
              "trait:CUSTOM_FPGA_INTEL_PAC_ARRIA10": "required",
              "trait:CUSTOM_FUNCTION_ID_3AFB": "required",
              },
@@ -58,7 +58,7 @@ def _get_device_profiles_as_dict():
         "updated_at": None,
         "description": "fake-daas_example_2-desc",
         "groups": [
-            {"resources:ACCELERATOR_FPGA": "1",
+            {"resources:FPGA": "1",
              "trait:CUSTOM_REGION_ID_3ACD": "required",
              "accel:bitstream_id": "ea0d149c-8555-495b-bc79-608d7bab1260"
              }

cyborg/tests/unit/objects/test_ext_arq_job.py

@@ -53,7 +53,7 @@ class TestExtARQJobMixin(base.DbTestCase):
             "device_profile_group"][constants.ACCEL_FUNCTION_ID]
 
     def test_get_resources_from_device_profile_group(self):
-        expect = [("GPU", 1)] + [("FPGA", 1)] * 4
+        expect = [("GPU")] + [("FPGA")] * 4
         actual = [v.get_resources_from_device_profile_group()
                   for v in self.class_objects.values()]
         self.assertEqual(expect, actual)