From 6f7cd8370ab42b10600ae73f6a9ac3b83d0daf20 Mon Sep 17 00:00:00 2001 From: Vitalii Kostenko Date: Tue, 3 Sep 2019 06:08:11 +0300 Subject: [PATCH] Ansible FTD Module improvements and tests update. (#60640) * Ansible FTD Module improvements and tests update. * Fix sanity tests * Fix formatting --- .../module_utils/network/ftd/common.py | 2 +- .../module_utils/network/ftd/configuration.py | 20 ++++++++- .../network/ftd/fdm_swagger_client.py | 10 +++-- .../network/ftd/test_configuration.py | 41 ++++++++++++++++++- .../module_utils/network/ftd/test_device.py | 2 +- .../network/ftd/test_upsert_functionality.py | 22 ++++++++-- test/units/plugins/httpapi/test_ftd.py | 1 - 7 files changed, 85 insertions(+), 13 deletions(-) diff --git a/lib/ansible/module_utils/network/ftd/common.py b/lib/ansible/module_utils/network/ftd/common.py index 0e330b3e6fb..de3f459d5b5 100644 --- a/lib/ansible/module_utils/network/ftd/common.py +++ b/lib/ansible/module_utils/network/ftd/common.py @@ -24,7 +24,7 @@ from ansible.module_utils.six import iteritems INVALID_IDENTIFIER_SYMBOLS = r'[^a-zA-Z0-9_]' IDENTITY_PROPERTIES = ['id', 'version', 'ruleId'] -NON_COMPARABLE_PROPERTIES = IDENTITY_PROPERTIES + ['isSystemDefined', 'links'] +NON_COMPARABLE_PROPERTIES = IDENTITY_PROPERTIES + ['isSystemDefined', 'links', 'token', 'rulePosition'] class HTTPMethod: diff --git a/lib/ansible/module_utils/network/ftd/configuration.py b/lib/ansible/module_utils/network/ftd/configuration.py index 4d92bdc0456..975bef379c0 100644 --- a/lib/ansible/module_utils/network/ftd/configuration.py +++ b/lib/ansible/module_utils/network/ftd/configuration.py @@ -208,6 +208,7 @@ class BaseConfigurationResource(object): self._models_operations_specs_cache = {} self._check_mode = check_mode self._operation_checker = OperationChecker + self._system_info = None def execute_operation(self, op_name, params): """ @@ -281,13 +282,30 @@ class BaseConfigurationResource(object): filters = params.get(ParamName.FILTERS) or {} if QueryParams.FILTER not in url_params[ParamName.QUERY_PARAMS] and 'name' in filters: # most endpoints only support filtering by name, so remaining `filters` are applied on returned objects - url_params[ParamName.QUERY_PARAMS][QueryParams.FILTER] = 'name:%s' % filters['name'] + url_params[ParamName.QUERY_PARAMS][QueryParams.FILTER] = self._stringify_name_filter(filters) item_generator = iterate_over_pageable_resource( partial(self.send_general_request, operation_name=operation_name), url_params ) return (i for i in item_generator if match_filters(filters, i)) + def _stringify_name_filter(self, filters): + build_version = self.get_build_version() + if build_version >= '6.4.0': + return "fts~%s" % filters['name'] + return "name:%s" % filters['name'] + + def _fetch_system_info(self): + if not self._system_info: + params = {ParamName.PATH_PARAMS: PATH_PARAMS_FOR_DEFAULT_OBJ} + self._system_info = self.send_general_request('getSystemInformation', params) + + return self._system_info + + def get_build_version(self): + system_info = self._fetch_system_info() + return system_info['databaseInfo']['buildVersion'] + def add_object(self, operation_name, params): def is_duplicate_name_error(err): return err.code == UNPROCESSABLE_ENTITY_STATUS and DUPLICATE_NAME_ERROR_MESSAGE in str(err) diff --git a/lib/ansible/module_utils/network/ftd/fdm_swagger_client.py b/lib/ansible/module_utils/network/ftd/fdm_swagger_client.py index ac5a2a5160c..9d0cd544611 100644 --- a/lib/ansible/module_utils/network/ftd/fdm_swagger_client.py +++ b/lib/ansible/module_utils/network/ftd/fdm_swagger_client.py @@ -78,6 +78,10 @@ class QueryParams: FILTER = 'filter' +class PathParams: + OBJ_ID = 'objId' + + def _get_model_name_from_url(schema_ref): path = schema_ref.split('/') return path[len(path) - 1] @@ -515,9 +519,9 @@ class FdmSwaggerValidator: def _is_enum(self, model): return self._is_string_type(model) and PropName.ENUM in model - def _check_enum(self, status, model, data, path): - if data is not None and data not in model[PropName.ENUM]: - self._add_invalid_type_report(status, path, '', PropName.ENUM, data) + def _check_enum(self, status, model, value, path): + if value is not None and value not in model[PropName.ENUM]: + self._add_invalid_type_report(status, path, '', PropName.ENUM, value) def _add_invalid_type_report(self, status, path, prop_name, expected_type, actually_value): status[PropName.INVALID_TYPE].append({ diff --git a/test/units/module_utils/network/ftd/test_configuration.py b/test/units/module_utils/network/ftd/test_configuration.py index df44f262c4d..2bc3de3ea28 100644 --- a/test/units/module_utils/network/ftd/test_configuration.py +++ b/test/units/module_utils/network/ftd/test_configuration.py @@ -40,13 +40,22 @@ class TestBaseConfigurationResource(object): return connection_instance + @patch.object(BaseConfigurationResource, '_fetch_system_info') @patch.object(BaseConfigurationResource, '_send_request') - def test_get_objects_by_filter_with_multiple_filters(self, send_request_mock, connection_mock): + def test_get_objects_by_filter_with_multiple_filters(self, send_request_mock, fetch_system_info_mock, + connection_mock): objects = [ {'name': 'obj1', 'type': 1, 'foo': {'bar': 'buzz'}}, {'name': 'obj2', 'type': 1, 'foo': {'bar': 'buz'}}, {'name': 'obj3', 'type': 2, 'foo': {'bar': 'buzz'}} ] + + fetch_system_info_mock.return_value = { + 'databaseInfo': { + 'buildVersion': '6.3.0' + } + } + connection_mock.get_operation_spec.return_value = { 'method': HTTPMethod.GET, 'url': '/object/' @@ -88,8 +97,10 @@ class TestBaseConfigurationResource(object): ] ) + @patch.object(BaseConfigurationResource, '_fetch_system_info') @patch.object(BaseConfigurationResource, '_send_request') - def test_get_objects_by_filter_with_multiple_responses(self, send_request_mock, connection_mock): + def test_get_objects_by_filter_with_multiple_responses(self, send_request_mock, fetch_system_info_mock, + connection_mock): send_request_mock.side_effect = [ {'items': [ {'name': 'obj1', 'type': 'foo'}, @@ -100,6 +111,11 @@ class TestBaseConfigurationResource(object): ]}, {'items': []} ] + fetch_system_info_mock.return_value = { + 'databaseInfo': { + 'buildVersion': '6.3.0' + } + } connection_mock.get_operation_spec.return_value = { 'method': HTTPMethod.GET, 'url': '/object/' @@ -296,6 +312,27 @@ class TestBaseConfigurationResource(object): 'invalid_type': [{'actually_value': 'test', 'expected_type': 'integer', 'path': 'f_integer'}], 'required': ['other_param']}} + @pytest.mark.parametrize("test_api_version, expected_result", + [ + ("6.2.3", "name:object_name"), + ("6.3.0", "name:object_name"), + ("6.4.0", "fts~object_name") + ] + ) + def test_stringify_name_filter(self, test_api_version, expected_result, connection_mock): + filters = {"name": "object_name"} + + with patch.object(BaseConfigurationResource, '_fetch_system_info') as fetch_system_info_mock: + fetch_system_info_mock.return_value = { + 'databaseInfo': { + 'buildVersion': test_api_version + } + } + resource = BaseConfigurationResource(connection_mock, False) + + assert resource._stringify_name_filter(filters) == expected_result, "Unexpected result for version %s" % ( + test_api_version) + class TestIterateOverPageableResource(object): diff --git a/test/units/module_utils/network/ftd/test_device.py b/test/units/module_utils/network/ftd/test_device.py index 426c836a0b1..3cbe0d61da7 100644 --- a/test/units/module_utils/network/ftd/test_device.py +++ b/test/units/module_utils/network/ftd/test_device.py @@ -78,7 +78,7 @@ class TestAbstractFtdPlatform(object): def test_parse_rommon_file_location_should_fail_for_non_tftp_protocol(self): with pytest.raises(ValueError) as ex: AbstractFtdPlatform.parse_rommon_file_location('http://1.2.3.4/boot/rommon-boot.foo') - assert 'The ROMMON image must be downloaded from TFTP server' in str(ex) + assert 'The ROMMON image must be downloaded from TFTP server' in str(ex.value) class TestFtd2100Platform(object): diff --git a/test/units/module_utils/network/ftd/test_upsert_functionality.py b/test/units/module_utils/network/ftd/test_upsert_functionality.py index 77ad791dd91..543ea8cea3f 100644 --- a/test/units/module_utils/network/ftd/test_upsert_functionality.py +++ b/test/units/module_utils/network/ftd/test_upsert_functionality.py @@ -39,9 +39,16 @@ ARBITRARY_RESPONSE = {'status': 'Arbitrary request sent'} class TestUpsertOperationUnitTests(unittest.TestCase): - def setUp(self): + + @mock.patch.object(BaseConfigurationResource, '_fetch_system_info') + def setUp(self, fetch_system_info_mock): self._conn = mock.MagicMock() self._resource = BaseConfigurationResource(self._conn) + fetch_system_info_mock.return_value = { + 'databaseInfo': { + 'buildVersion': '6.3.0' + } + } def test_get_operation_name(self): operation_a = mock.MagicMock() @@ -856,10 +863,17 @@ class TestUpsertOperationFunctionalTests(object): @staticmethod def _resource_execute_operation(params, connection): - resource = BaseConfigurationResource(connection) - op_name = params['operation'] - resp = resource.execute_operation(op_name, params) + with mock.patch.object(BaseConfigurationResource, '_fetch_system_info') as fetch_system_info_mock: + fetch_system_info_mock.return_value = { + 'databaseInfo': { + 'buildVersion': '6.3.0' + } + } + resource = BaseConfigurationResource(connection) + op_name = params['operation'] + + resp = resource.execute_operation(op_name, params) return resp diff --git a/test/units/plugins/httpapi/test_ftd.py b/test/units/plugins/httpapi/test_ftd.py index 51c5fd605ae..7767ea2db8b 100644 --- a/test/units/plugins/httpapi/test_ftd.py +++ b/test/units/plugins/httpapi/test_ftd.py @@ -1,5 +1,4 @@ # Copyright (c) 2018 Cisco and/or its affiliates. -# Copyright (c) 2018 Cisco and/or its affiliates. # # This file is part of Ansible #