From 26638f6f4f059e9fcea64e078a587b10384ec777 Mon Sep 17 00:00:00 2001 From: Ron Erez Date: Mon, 7 Jul 2025 11:57:45 +0300 Subject: [PATCH 1/5] Fix advanced mapping merge behavior and add comprehensive test - Fix set_advanced_mapping to properly merge with existing mappings - Add test for existing mappings preservation - Maintain SDK consistency with other configuration methods --- cterasdk/edge/directoryservice.py | 7 +++-- tests/ut/edge/test_directory_service.py | 42 ++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/cterasdk/edge/directoryservice.py b/cterasdk/edge/directoryservice.py index c458c4f2..7a98d100 100644 --- a/cterasdk/edge/directoryservice.py +++ b/cterasdk/edge/directoryservice.py @@ -108,13 +108,14 @@ def set_advanced_mapping(self, mappings): raise CTERAException('Failed to configure advanced mapping. Not connected to directory services.') domains = self.domains() - advanced_mapping = self._edge.api.get('/config/fileservices/cifs/idMapping/map') - advanced_mapping = [] + existing_mappings = self._edge.api.get('/config/fileservices/cifs/idMapping/map') + mapping_dict = {mapping.domainFlatName: mapping for mapping in existing_mappings} for mapping in mappings: if mapping.domainFlatName in domains: - advanced_mapping.append(mapping) + mapping_dict[mapping.domainFlatName] = mapping else: logger.warning('Invalid mapping. Could not find domain. %s', {'domain': mapping.domainFlatName}) + advanced_mapping = list(mapping_dict.values()) logger.debug('Updating advanced mapping. %s', { 'domains': [mapping.domainFlatName for mapping in advanced_mapping] diff --git a/tests/ut/edge/test_directory_service.py b/tests/ut/edge/test_directory_service.py index d07e8ed9..c3c848e7 100644 --- a/tests/ut/edge/test_directory_service.py +++ b/tests/ut/edge/test_directory_service.py @@ -17,8 +17,11 @@ def setUp(self): self._password = 'password' self._workgroup = "CTERA" self._domain_flat_name = "CTERA" + self._domain_flat_name_2 = "MYDOMAIN" self._mapping_min = 2000000 self._mapping_max = 5000000 + self._mapping_min_2 = 1000000 + self._mapping_max_2 = 1500000 self._dc = '192.168.0.1' self._ports = [389, 3268, 445] @@ -144,7 +147,10 @@ def test_set_advanced_mapping(self): advanced_mapping = [ TestEdgeDirectoryService._get_advanced_mapping_object(self._domain_flat_name, self._mapping_min, self._mapping_max) ] - self._init_filer(get_response=0, execute_response=execute_response) + existing_mappings = [] + get_response_side_effect = TestEdgeDirectoryService._get_set_advanced_mapping_side_effect(0, existing_mappings) + self._init_filer(execute_response=execute_response) + self._filer.api.get = mock.MagicMock(side_effect=get_response_side_effect) directoryservice.DirectoryService(self._filer).set_advanced_mapping(advanced_mapping) self._filer.api.get.assert_has_calls([ mock.call('/status/fileservices/cifs/joinStatus'), @@ -155,6 +161,30 @@ def test_set_advanced_mapping(self): actual_param = self._filer.api.put.call_args[0][1] self._assert_equal_objects(advanced_mapping[0], actual_param[0]) + def test_set_advanced_mapping_with_existing_mappings(self): + execute_response = TestEdgeDirectoryService._create_get_domains_response(self._domain_flat_name, self._domain_flat_name_2) + advanced_mapping = [ + TestEdgeDirectoryService._get_advanced_mapping_object(self._domain_flat_name, self._mapping_min, self._mapping_max) + ] + existing_mappings = [ + TestEdgeDirectoryService._get_advanced_mapping_object(self._domain_flat_name_2, self._mapping_min_2, self._mapping_max_2) + ] + get_response_side_effect = TestEdgeDirectoryService._get_set_advanced_mapping_side_effect(0, existing_mappings) + self._init_filer(execute_response=execute_response) + self._filer.api.get = mock.MagicMock(side_effect=get_response_side_effect) + directoryservice.DirectoryService(self._filer).set_advanced_mapping(advanced_mapping) + self._filer.api.get.assert_has_calls([ + mock.call('/status/fileservices/cifs/joinStatus'), + mock.call('/config/fileservices/cifs/idMapping/map') + ]) + self._filer.api.execute.assert_called_once_with('/status/fileservices/cifs', 'enumDiscoveredDomains') + self._filer.api.put.assert_called_once_with('/config/fileservices/cifs/idMapping/map', mock.ANY) + actual_param = self._filer.api.put.call_args[0][1] + self.assertEqual(len(actual_param), 2) + domain_names = [mapping.domainFlatName for mapping in actual_param] + self.assertIn(self._domain_flat_name, domain_names) + self.assertIn(self._domain_flat_name_2, domain_names) + def test_set_advanced_mapping_raise(self): self._init_filer(get_response=1) with self.assertRaises(exceptions.CTERAException) as error: @@ -253,3 +283,13 @@ def get_response(path): return cifs_param return None return get_response + + @staticmethod + def _get_set_advanced_mapping_side_effect(join_status, existing_mappings): + def get_response(path): + if path == '/status/fileservices/cifs/joinStatus': + return join_status + if path == '/config/fileservices/cifs/idMapping/map': + return existing_mappings + return None + return get_response From 8baecb048c4f472db83b0b25ade3f842fdc5dd22 Mon Sep 17 00:00:00 2001 From: Saimon Michelson Date: Mon, 7 Jul 2025 18:33:29 -0400 Subject: [PATCH 2/5] revert implementation, remove redundant call to retrieve mappings --- cterasdk/edge/directoryservice.py | 6 ++-- tests/ut/edge/test_directory_service.py | 47 ++----------------------- 2 files changed, 4 insertions(+), 49 deletions(-) diff --git a/cterasdk/edge/directoryservice.py b/cterasdk/edge/directoryservice.py index 7a98d100..82c33f45 100644 --- a/cterasdk/edge/directoryservice.py +++ b/cterasdk/edge/directoryservice.py @@ -108,14 +108,12 @@ def set_advanced_mapping(self, mappings): raise CTERAException('Failed to configure advanced mapping. Not connected to directory services.') domains = self.domains() - existing_mappings = self._edge.api.get('/config/fileservices/cifs/idMapping/map') - mapping_dict = {mapping.domainFlatName: mapping for mapping in existing_mappings} + advanced_mapping = [] for mapping in mappings: if mapping.domainFlatName in domains: - mapping_dict[mapping.domainFlatName] = mapping + advanced_mapping.append(mapping) else: logger.warning('Invalid mapping. Could not find domain. %s', {'domain': mapping.domainFlatName}) - advanced_mapping = list(mapping_dict.values()) logger.debug('Updating advanced mapping. %s', { 'domains': [mapping.domainFlatName for mapping in advanced_mapping] diff --git a/tests/ut/edge/test_directory_service.py b/tests/ut/edge/test_directory_service.py index c3c848e7..1a145e3b 100644 --- a/tests/ut/edge/test_directory_service.py +++ b/tests/ut/edge/test_directory_service.py @@ -17,11 +17,8 @@ def setUp(self): self._password = 'password' self._workgroup = "CTERA" self._domain_flat_name = "CTERA" - self._domain_flat_name_2 = "MYDOMAIN" self._mapping_min = 2000000 self._mapping_max = 5000000 - self._mapping_min_2 = 1000000 - self._mapping_max_2 = 1500000 self._dc = '192.168.0.1' self._ports = [389, 3268, 445] @@ -147,44 +144,14 @@ def test_set_advanced_mapping(self): advanced_mapping = [ TestEdgeDirectoryService._get_advanced_mapping_object(self._domain_flat_name, self._mapping_min, self._mapping_max) ] - existing_mappings = [] - get_response_side_effect = TestEdgeDirectoryService._get_set_advanced_mapping_side_effect(0, existing_mappings) - self._init_filer(execute_response=execute_response) - self._filer.api.get = mock.MagicMock(side_effect=get_response_side_effect) + self._init_filer(get_response=0, execute_response=execute_response) directoryservice.DirectoryService(self._filer).set_advanced_mapping(advanced_mapping) - self._filer.api.get.assert_has_calls([ - mock.call('/status/fileservices/cifs/joinStatus'), - mock.call('/config/fileservices/cifs/idMapping/map') - ]) + self._filer.api.get.assert_called_once_with('/status/fileservices/cifs/joinStatus') self._filer.api.execute.assert_called_once_with('/status/fileservices/cifs', 'enumDiscoveredDomains') self._filer.api.put.assert_called_once_with('/config/fileservices/cifs/idMapping/map', mock.ANY) actual_param = self._filer.api.put.call_args[0][1] self._assert_equal_objects(advanced_mapping[0], actual_param[0]) - def test_set_advanced_mapping_with_existing_mappings(self): - execute_response = TestEdgeDirectoryService._create_get_domains_response(self._domain_flat_name, self._domain_flat_name_2) - advanced_mapping = [ - TestEdgeDirectoryService._get_advanced_mapping_object(self._domain_flat_name, self._mapping_min, self._mapping_max) - ] - existing_mappings = [ - TestEdgeDirectoryService._get_advanced_mapping_object(self._domain_flat_name_2, self._mapping_min_2, self._mapping_max_2) - ] - get_response_side_effect = TestEdgeDirectoryService._get_set_advanced_mapping_side_effect(0, existing_mappings) - self._init_filer(execute_response=execute_response) - self._filer.api.get = mock.MagicMock(side_effect=get_response_side_effect) - directoryservice.DirectoryService(self._filer).set_advanced_mapping(advanced_mapping) - self._filer.api.get.assert_has_calls([ - mock.call('/status/fileservices/cifs/joinStatus'), - mock.call('/config/fileservices/cifs/idMapping/map') - ]) - self._filer.api.execute.assert_called_once_with('/status/fileservices/cifs', 'enumDiscoveredDomains') - self._filer.api.put.assert_called_once_with('/config/fileservices/cifs/idMapping/map', mock.ANY) - actual_param = self._filer.api.put.call_args[0][1] - self.assertEqual(len(actual_param), 2) - domain_names = [mapping.domainFlatName for mapping in actual_param] - self.assertIn(self._domain_flat_name, domain_names) - self.assertIn(self._domain_flat_name_2, domain_names) - def test_set_advanced_mapping_raise(self): self._init_filer(get_response=1) with self.assertRaises(exceptions.CTERAException) as error: @@ -283,13 +250,3 @@ def get_response(path): return cifs_param return None return get_response - - @staticmethod - def _get_set_advanced_mapping_side_effect(join_status, existing_mappings): - def get_response(path): - if path == '/status/fileservices/cifs/joinStatus': - return join_status - if path == '/config/fileservices/cifs/idMapping/map': - return existing_mappings - return None - return get_response From 426758cb4715b2c8fe9c5349e7e8abd6c279e0ab Mon Sep 17 00:00:00 2001 From: Saimon Michelson Date: Mon, 7 Jul 2025 18:35:30 -0400 Subject: [PATCH 3/5] update docs --- docs/source/UserGuides/Edge/Configuration.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/source/UserGuides/Edge/Configuration.rst b/docs/source/UserGuides/Edge/Configuration.rst index 16b611e6..716af95d 100644 --- a/docs/source/UserGuides/Edge/Configuration.rst +++ b/docs/source/UserGuides/Edge/Configuration.rst @@ -449,6 +449,8 @@ Active Directory edge.directoryservice.set_advanced_mapping(advanced_mapping) # this function will skip domains that are not found +.. note:: This method sets the advanced mapping of domains and requires the full list of mappings. + It does not append to the existing configuration—any existing mappings will be overwritten. .. note:: to retrieve a list of domain flat names, use :py:func:`cterasdk.edge.directoryservice.domains()` .. automethod:: cterasdk.edge.directoryservice.DirectoryService.disconnect From eac892e3a1824e5f05b74107316d679b5e35929a Mon Sep 17 00:00:00 2001 From: Saimon Michelson Date: Mon, 7 Jul 2025 18:36:55 -0400 Subject: [PATCH 4/5] update changelog --- docs/source/UserGuides/Miscellaneous/Changelog.rst | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/source/UserGuides/Miscellaneous/Changelog.rst b/docs/source/UserGuides/Miscellaneous/Changelog.rst index 73c52757..59ad3fa2 100644 --- a/docs/source/UserGuides/Miscellaneous/Changelog.rst +++ b/docs/source/UserGuides/Miscellaneous/Changelog.rst @@ -8,8 +8,15 @@ Improvements ^^^^^^^^^^^^ * Added support for enabling or disabling Direct Mode on CTERA Portal Storage Nodes. +* Introduced a method for setting the Advanced Domain Mapping configuration. **Note:** This method requires the full list of mappings to be provided and will overwrite the existing configuration. -Related issues and pull requests on GitHub: `#310 `_ +Bug Fixes +^^^^^^^^^ + +* Removed redundant call when retrieving the list of domain mappings. + +Related issues and pull requests on GitHub: `#310 `_, +`#311 `_ 2.20.15 ------- From 73236f7fd997b0df99a75c7043c3851a22ee510a Mon Sep 17 00:00:00 2001 From: Saimon Michelson Date: Mon, 7 Jul 2025 18:42:40 -0400 Subject: [PATCH 5/5] update changelog --- docs/source/UserGuides/Miscellaneous/Changelog.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/source/UserGuides/Miscellaneous/Changelog.rst b/docs/source/UserGuides/Miscellaneous/Changelog.rst index 59ad3fa2..d44c94b2 100644 --- a/docs/source/UserGuides/Miscellaneous/Changelog.rst +++ b/docs/source/UserGuides/Miscellaneous/Changelog.rst @@ -8,7 +8,6 @@ Improvements ^^^^^^^^^^^^ * Added support for enabling or disabling Direct Mode on CTERA Portal Storage Nodes. -* Introduced a method for setting the Advanced Domain Mapping configuration. **Note:** This method requires the full list of mappings to be provided and will overwrite the existing configuration. Bug Fixes ^^^^^^^^^