From 7add03725937d6bc8ea7a975f868ea877fe7b5e2 Mon Sep 17 00:00:00 2001 From: Tsukasa Inoue Date: Tue, 8 Jun 2021 12:12:21 +0900 Subject: [PATCH] Fix status code for invalid filter conditions The current tacker returns 500 Internal Server Error to requests with invalid filter conditions where a VNFID is not UUID. However, as the error is caused by a bad request, the status code should be 400. This bug occurs because data types of 'id' and 'vnfd_Id' in TackerObject are 'string' which don't match the data types in DB, i.e., UUID. In order to fix this bug, those data types in TackerObject are changed to 'uuid'. Closes-Bug: 1924666 Closes-Bug: 1926632 Change-Id: I8decbb42d661a44c6c382cb1abbfc37d95084ffb --- tacker/api/common/_filters.py | 22 ++++++++++--------- tacker/objects/vnf_instance.py | 6 ++--- tacker/objects/vnf_instantiated_info.py | 2 +- tacker/objects/vnf_lcm_op_occs.py | 2 +- tacker/objects/vnf_lcm_subscriptions.py | 2 +- tacker/objects/vnf_package.py | 4 ++-- tacker/objects/vnf_software_image.py | 2 +- tacker/tests/unit/vnflcm/test_controller.py | 7 +++--- tacker/tests/unit/vnfpkgm/test_controller.py | 23 +++++++++++--------- 9 files changed, 38 insertions(+), 32 deletions(-) diff --git a/tacker/api/common/_filters.py b/tacker/api/common/_filters.py index f887f1ebc..b0ef68f72 100644 --- a/tacker/api/common/_filters.py +++ b/tacker/api/common/_filters.py @@ -67,16 +67,18 @@ class Filter(BaseFilter): } OPERATOR_SUPPORTED_DATA_TYPES = { - 'eq': ['string', 'number', 'enum', 'boolean', 'key_value_pair'], - 'neq': ['string', 'number', 'enum', 'boolean', 'key_value_pair'], - 'in': ['string', 'number', 'enum', 'key_value_pair'], - 'nin': ['string', 'number', 'enum', 'key_value_pair'], - 'gt': ['string', 'number', 'datetime', 'key_value_pair'], - 'gte': ['string', 'number', 'datetime', 'key_value_pair'], - 'lt': ['string', 'number', 'datetime', 'key_value_pair'], - 'lte': ['string', 'number', 'datetime', 'key_value_pair'], - 'cont': ['string', 'key_value_pair'], - 'ncont': ['string', 'key_value_pair'], + 'eq': ['uuid', 'string', 'number', 'enum', 'boolean', + 'key_value_pair'], + 'neq': ['uuid', 'string', 'number', 'enum', 'boolean', + 'key_value_pair'], + 'in': ['uuid', 'string', 'number', 'enum', 'key_value_pair'], + 'nin': ['uuid', 'string', 'number', 'enum', 'key_value_pair'], + 'gt': ['uuid', 'string', 'number', 'datetime', 'key_value_pair'], + 'gte': ['uuid', 'string', 'number', 'datetime', 'key_value_pair'], + 'lt': ['uuid', 'string', 'number', 'datetime', 'key_value_pair'], + 'lte': ['uuid', 'string', 'number', 'datetime', 'key_value_pair'], + 'cont': ['uuid', 'string', 'key_value_pair'], + 'ncont': ['uuid', 'string', 'key_value_pair'], } def __init__(self, operator, attribute, values): diff --git a/tacker/objects/vnf_instance.py b/tacker/objects/vnf_instance.py index 4fccc5dc2..2ba04843c 100644 --- a/tacker/objects/vnf_instance.py +++ b/tacker/objects/vnf_instance.py @@ -334,20 +334,20 @@ class VnfInstance(base.TackerObject, base.TackerPersistentObject, } ALL_ATTRIBUTES = { - 'id': ('id', "string", 'VnfInstance'), + 'id': ('id', "uuid", 'VnfInstance'), 'vnfInstanceName': ('vnf_instance_name', 'string', 'VnfInstance'), 'vnfInstanceDescription': ( 'vnf_instance_description', 'string', 'VnfInstance'), 'instantiationState': ('instantiation_state', 'string', 'VnfInstance'), 'taskState': ('task_state', 'string', 'VnfInstance'), - 'vnfdId': ('vnfd_id', 'string', 'VnfInstance'), + 'vnfdId': ('vnfd_id', 'uuid', 'VnfInstance'), 'vnfProvider': ('vnf_provider', 'string', 'VnfInstance'), 'vnfProductName': ('vnf_product_name', 'string', 'VnfInstance'), 'vnfSoftwareVersion': ( 'vnf_software_version', 'string', 'VnfInstance'), 'vnfdVersion': ('vnfd_version', 'string', 'VnfInstance'), 'tenantId': ('tenant_id', 'string', 'VnfInstance'), - 'vnfPkgId': ('vnf_pkg_id', 'string', 'VnfInstance'), + 'vnfPkgId': ('vnf_pkg_id', 'uuid', 'VnfInstance'), 'vimConnectionInfo/*': ('vim_connection_info', 'key_value_pair', {"key_column": "key", "value_column": "value", "model": "VnfInstance"}), diff --git a/tacker/objects/vnf_instantiated_info.py b/tacker/objects/vnf_instantiated_info.py index 29169b6ae..f6ead2af5 100644 --- a/tacker/objects/vnf_instantiated_info.py +++ b/tacker/objects/vnf_instantiated_info.py @@ -101,7 +101,7 @@ class InstantiatedVnfInfo(base.TackerObject, base.TackerObjectDictCompat, 'instantiatedInfo': { 'flavourId': ('id', 'string', 'VnfInstantiatedInfo'), 'vnfInstanceId': - ('vnf_instance_id', 'string', 'VnfInstantiatedInfo'), + ('vnf_instance_id', 'uuid', 'VnfInstantiatedInfo'), 'vnfState': ('vnf_state', 'string', 'VnfInstantiatedInfo'), 'instanceId': ('instance_id', 'string', 'VnfInstantiatedInfo'), 'instantiationLevelId': diff --git a/tacker/objects/vnf_lcm_op_occs.py b/tacker/objects/vnf_lcm_op_occs.py index 4c5e5966b..d8c8fc197 100644 --- a/tacker/objects/vnf_lcm_op_occs.py +++ b/tacker/objects/vnf_lcm_op_occs.py @@ -246,7 +246,7 @@ class VnfLcmOpOcc(base.TackerObject, base.TackerObjectDictCompat, } ALL_ATTRIBUTES = { - 'id': ('id', 'string', 'VnfLcmOpOccs'), + 'id': ('id', 'uuid', 'VnfLcmOpOccs'), 'operationState': ('operation_state', 'string', 'VnfLcmOpOccs'), 'stateEnteredTime': ('state_entered_time', 'datetime', 'VnfLcmOpOccs'), diff --git a/tacker/objects/vnf_lcm_subscriptions.py b/tacker/objects/vnf_lcm_subscriptions.py index 8e4043be1..628023e87 100644 --- a/tacker/objects/vnf_lcm_subscriptions.py +++ b/tacker/objects/vnf_lcm_subscriptions.py @@ -594,7 +594,7 @@ class LccnSubscription(base.TackerObject, base.TackerPersistentObject): } ALL_ATTRIBUTES = { - 'id': ('id', 'string', + 'id': ('id', 'uuid', 'VnfLcmSubscriptions'), 'vnfdIds': ('vnfd_ids', 'string', 'VnfLcmFilters'), diff --git a/tacker/objects/vnf_package.py b/tacker/objects/vnf_package.py index 32662c8a0..5d1eeebec 100644 --- a/tacker/objects/vnf_package.py +++ b/tacker/objects/vnf_package.py @@ -286,7 +286,7 @@ class VnfPackage(base.TackerObject, base.TackerPersistentObject, # 4. Valid values for a given data type if any. This value is set # especially for 'enum' data type. ALL_ATTRIBUTES = { - 'id': ('id', "string", 'VnfPackage'), + 'id': ('id', "uuid", 'VnfPackage'), 'onboardingState': ('onboarding_state', "enum", 'VnfPackage', fields.PackageOnboardingStateTypeField().valid_values), 'operationalState': ('operational_state', 'enum', 'VnfPackage', @@ -296,7 +296,7 @@ class VnfPackage(base.TackerObject, base.TackerPersistentObject, 'vnfProvider': ('vnfd.vnf_provider', 'string', 'VnfPackageVnfd'), 'vnfProductName': ('vnfd.vnf_product_name', 'string', 'VnfPackageVnfd'), - 'vnfdId': ('vnfd.vnfd_id', 'string', 'VnfPackageVnfd'), + 'vnfdId': ('vnfd.vnfd_id', 'uuid', 'VnfPackageVnfd'), 'vnfSoftwareVersion': ('vnfd.vnf_software_version', 'string', 'VnfPackageVnfd'), 'vnfdVersion': ('vnfd.vnfd_version', 'string', 'VnfPackageVnfd'), diff --git a/tacker/objects/vnf_software_image.py b/tacker/objects/vnf_software_image.py index 8dea9b62d..5ae090fc4 100644 --- a/tacker/objects/vnf_software_image.py +++ b/tacker/objects/vnf_software_image.py @@ -83,7 +83,7 @@ class VnfSoftwareImage(base.TackerObject, base.TackerPersistentObject): ALL_ATTRIBUTES = { "softwareImages": { - 'id': ('software_image_id', 'string', 'VnfSoftwareImage'), + 'id': ('software_image_id', 'uuid', 'VnfSoftwareImage'), 'imagePath': ('image_path', 'string', 'VnfSoftwareImage'), 'diskFormat': ('disk_format', 'string', 'VnfSoftwareImage'), 'userMetadata/*': ('metadata', 'key_value_pair', diff --git a/tacker/tests/unit/vnflcm/test_controller.py b/tacker/tests/unit/vnflcm/test_controller.py index 20a7e1612..04e8ea6c6 100644 --- a/tacker/tests/unit/vnflcm/test_controller.py +++ b/tacker/tests/unit/vnflcm/test_controller.py @@ -1887,16 +1887,17 @@ class TestController(base.TestCase): {'filter': "(eq,vnfInstanceDescription,dummy value)"}, {'filter': "(eq,instantiationState,'NOT_INSTANTIATED')"}, {'filter': "(eq,taskState,'ACTIVE')"}, - {'filter': "(eq,vnfdId,'dummy_vnfd_id')"}, + {'filter': "(eq,vnfdId,%s)" % uuidsentinel.vnfd_id}, {'filter': "(eq,vnfProvider,'''dummy ''hi'' value''')"}, {'filter': "(eq,vnfProductName,'dummy_product_name')"}, {'filter': "(eq,vnfSoftwareVersion,'1.0')"}, {'filter': "(eq,vnfdVersion,'dummy_vnfd_version')"}, {'filter': "(eq,tenantId,'dummy_tenant_id')"}, - {'filter': "(eq,vnfPkgId,'dummy_pkg_id')"}, + {'filter': "(eq,vnfPkgId,%s)" % uuidsentinel.vnf_pkg_id}, {'filter': "(eq,vimConnectionInfo/accessInfo/region,'dummy_id')"}, {'filter': "(eq,instantiatedInfo/flavourId,'dummy_flavour')"}, - {'filter': "(eq,instantiatedInfo/vnfInstanceId,'dummy_vnf_id')"}, + {'filter': "(eq,instantiatedInfo/vnfInstanceId,%s)" % + uuidsentinel.vnf_instance_id}, {'filter': "(eq,instantiatedInfo/vnfState,'ACTIVE')"}, {'filter': "(eq,instantiatedInfo/instanceId,'dummy_vnf_id')"}, {'filter': diff --git a/tacker/tests/unit/vnfpkgm/test_controller.py b/tacker/tests/unit/vnfpkgm/test_controller.py index 8aec64781..33fea8962 100644 --- a/tacker/tests/unit/vnfpkgm/test_controller.py +++ b/tacker/tests/unit/vnfpkgm/test_controller.py @@ -262,12 +262,12 @@ class TestController(base.TestCase): @mock.patch.object(VnfPackagesList, "get_by_filters") @ddt.data( - {'filter': '(eq,vnfdId,dummy_vnfd_id)'}, - {'filter': '(in,vnfdId,dummy_vnfd_id)'}, - {'filter': '(cont,vnfdId,dummy_vnfd_id)'}, - {'filter': '(neq,vnfdId,dummy_vnfd_id)'}, - {'filter': '(nin,vnfdId,dummy_vnfd_id)'}, - {'filter': '(ncont,vnfdId,dummy_vnfd_id)'}, + {'filter': '(eq,vnfdId,%s)' % constants.UUID}, + {'filter': '(in,vnfdId,%s)' % constants.UUID}, + {'filter': '(cont,vnfdId,%s)' % constants.UUID}, + {'filter': '(neq,vnfdId,%s)' % constants.UUID}, + {'filter': '(nin,vnfdId,%s)' % constants.UUID}, + {'filter': '(ncont,vnfdId,%s)' % constants.UUID}, {'filter': '(gt,softwareImages/createdAt,2020-03-11 04:10:15+00:00)'}, {'filter': '(gte,softwareImages/createdAt,2020-03-14 04:10:15+00:00)'}, {'filter': '(lt,softwareImages/createdAt,2020-03-20 04:10:15+00:00)'}, @@ -302,7 +302,8 @@ class TestController(base.TestCase): @mock.patch.object(VnfPackagesList, "get_by_filters") def test_index_filter_combination(self, mock_vnf_list): """Test multiple filter parameters separated by semicolon """ - params = {'filter': '(eq,vnfdId,dummy_vnfd_id);(eq,id,dummy_id)'} + params = {'filter': '(eq,vnfdId,%s);(eq,id,%s)' % + (constants.UUID, constants.UUID)} query = urllib.parse.urlencode(params) req = fake_request.HTTPRequest.blank('/vnfpkgm/v1/vnf_packages?' + query) @@ -318,8 +319,8 @@ class TestController(base.TestCase): @mock.patch.object(VnfPackagesList, "get_by_filters") @ddt.data( - {'filter': '(eq,id,dummy_value)'}, - {'filter': '(eq,vnfdId,dummy_value)'}, + {'filter': '(eq,id,%s)' % constants.UUID}, + {'filter': '(eq,vnfdId,%s)' % constants.UUID}, {'filter': '(eq,onboardingState,ONBOARDED)'}, {'filter': '(eq,operationalState,ENABLED)'}, {'filter': '(eq,usageState,NOT_IN_USE)'}, @@ -330,7 +331,7 @@ class TestController(base.TestCase): {'filter': '(eq,userDefinedData/key1,dummy_value)'}, {'filter': '(eq,checksum/algorithm,dummy_value)'}, {'filter': '(eq,checksum/hash,dummy_value)'}, - {'filter': '(eq,softwareImages/id,dummy_value)'}, + {'filter': '(eq,softwareImages/id,%s)' % constants.UUID}, {'filter': '(eq,softwareImages/imagePath,dummy_value)'}, {'filter': '(eq,softwareImages/diskFormat,dummy_value)'}, {'filter': '(eq,softwareImages/userMetadata/key3,dummy_value)'}, @@ -464,6 +465,8 @@ class TestController(base.TestCase): @mock.patch.object(VnfPackagesList, "get_by_filters") @ddt.data( + {'filter': '(eq,id,fake_value)'}, + {'filter': '(eq,vnfd_id,fake_value)'}, {'filter': '(eq,operationalState,fake_value)'}, {'filter': '(eq,softwareImages/size,fake_value)'}, {'filter': '(gt,softwareImages/createdAt,fake_value)'},