diff --git a/vmware_nsxlib/tests/unit/v3/policy/test_resources.py b/vmware_nsxlib/tests/unit/v3/policy/test_resources.py index b30d5905..5d3420ed 100644 --- a/vmware_nsxlib/tests/unit/v3/policy/test_resources.py +++ b/vmware_nsxlib/tests/unit/v3/policy/test_resources.py @@ -1982,7 +1982,7 @@ class TestPolicyCommunicationMap(NsxPolicyLibTestCase): 'display_name': 'map_name', 'rules': [ {'id': entry1_id, 'resource_type': 'Rule', - 'dsiplay_name': 'name1', 'scope': ['scope1']}, + 'display_name': 'name1', 'scope': ['scope1']}, {'id': entry2_id, 'resource_type': 'Rule', 'display_name': 'name2', 'scope': ['scope2']}, {'id': entry3_id, 'resource_type': 'Rule', @@ -1994,7 +1994,7 @@ class TestPolicyCommunicationMap(NsxPolicyLibTestCase): 'display_name': 'new_map_name', 'rules': [ {'id': entry1_id, 'resource_type': 'Rule', - 'dsiplay_name': 'name1', 'scope': ['new_scope1']}, + 'display_name': 'name1', 'scope': ['new_scope1']}, {'id': entry2_id, 'resource_type': 'Rule', 'display_name': 'name2', 'scope': ['scope2']}]} map_def = self.mapDef( @@ -2011,6 +2011,51 @@ class TestPolicyCommunicationMap(NsxPolicyLibTestCase): update_call.assert_called_once_with( map_def.get_resource_path(), updated_map) + def test_update_with_entries_for_IGNORE_entries(self): + domain_id = '111' + map_id = '222' + entry1_id = 'entry1' + entry2_id = 'entry2' + entry3_id = 'entry3' + original_map = { + 'id': map_id, + 'resource_type': self.resource_type, + 'category': constants.CATEGORY_APPLICATION, + 'display_name': 'map_name', + 'rules': [ + {'id': entry1_id, 'resource_type': 'Rule', + 'display_name': 'name1', 'scope': ['scope1'], + '_created_time': 1}, + {'id': entry2_id, 'resource_type': 'Rule', + 'display_name': 'name2', 'scope': ['scope2']}, + {'id': entry3_id, 'resource_type': 'Rule', + 'display_name': 'name3', 'scope': ['scope3']}]} + updated_map = { + 'id': map_id, + 'resource_type': self.resource_type, + 'category': constants.CATEGORY_APPLICATION, + 'display_name': 'new_map_name', + 'rules': [ + {'id': entry1_id, 'resource_type': 'Rule', + 'display_name': 'name1', 'scope': ['scope1'], + '_created_time': 1}, + {'id': entry2_id, 'resource_type': 'Rule', + 'display_name': 'name2', 'scope': ['scope2']}, + {'id': entry3_id, 'resource_type': 'Rule', + 'display_name': 'name3', 'scope': ['scope3']}]} + map_def = self.mapDef( + domain_id=domain_id, + map_id=map_id, + tenant=TEST_TENANT) + with mock.patch.object(self.policy_api, "get", + return_value=original_map),\ + mock.patch.object(self.policy_api.client, + "update") as update_call: + self.resourceApi.update_with_entries( + domain_id, map_id, name='new_map_name', tenant=TEST_TENANT) + update_call.assert_called_once_with( + map_def.get_resource_path(), updated_map) + def test_unset(self): name = 'hello' domain_id = 'test' diff --git a/vmware_nsxlib/tests/unit/v3/policy/test_transaction.py b/vmware_nsxlib/tests/unit/v3/policy/test_transaction.py index acf3c951..67fb387b 100644 --- a/vmware_nsxlib/tests/unit/v3/policy/test_transaction.py +++ b/vmware_nsxlib/tests/unit/v3/policy/test_transaction.py @@ -14,6 +14,8 @@ # under the License. # +import copy + import mock from vmware_nsxlib.tests.unit.v3 import nsxlib_testcase @@ -248,3 +250,189 @@ class TestPolicyTransaction(policy_testcase.TestPolicyApi): 'Segment': seg2}]} self.assert_infra_patch_call(expected_body) + + def test_creating_security_policy_and_dfw_rules(self): + dfw_rule = {'id': 'rule_id1', 'action': 'ALLOW', + 'display_name': 'rule1', 'description': None, + 'direction': 'IN_OUT', 'ip_protocol': 'IPV4_IPV6', + 'logged': False, 'destination_groups': ['destination_url'], + 'source_groups': ['src_url'], 'resource_type': 'Rule', + 'scope': None, 'sequence_number': None, 'tag': None, + 'services': ['ANY']} + security_policy = {'id': 'security_policy_id1', + 'display_name': 'security_policy', + 'category': 'Application', + 'resource_type': 'SecurityPolicy'} + domain = {'resource_type': 'Domain', 'id': 'domain1'} + domain_id = domain['id'] + map_id = security_policy['id'] + dfw_rule_entries = [self.policy_lib.comm_map.build_entry( + name=dfw_rule['display_name'], + domain_id=domain_id, + map_id=map_id, + entry_id=dfw_rule['id'], + source_groups=dfw_rule['source_groups'], + dest_groups=dfw_rule['destination_groups'] + )] + with trans.NsxPolicyTransaction(): + self.policy_lib.comm_map.create_with_entries( + name=security_policy['display_name'], + domain_id=domain_id, + map_id=map_id, + entries=dfw_rule_entries + ) + + def get_group_path(group_id, domain_id): + return '/infra/domains/' + domain_id + '/groups/' + group_id + + dfw_rule['destination_groups'] = [get_group_path(group_id, domain_id) + for group_id in + dfw_rule['destination_groups']] + dfw_rule['source_groups'] = [get_group_path(group_id, domain_id) for + group_id in dfw_rule['source_groups']] + child_rules = [{'resource_type': 'ChildRule', 'Rule': dfw_rule}] + security_policy.update({'children': child_rules}) + child_security_policies = [{ + 'resource_type': 'ChildSecurityPolicy', + 'SecurityPolicy': security_policy + }] + domain.update({'children': child_security_policies}) + child_domains = [{'resource_type': 'ChildDomain', + 'Domain': domain}] + expected_body = {'resource_type': 'Infra', + 'children': child_domains} + self.assert_infra_patch_call(expected_body) + + @mock.patch('vmware_nsxlib.v3.policy.core_defs.NsxPolicyApi.get') + def test_updating_security_policy_and_dfw_rules(self, mock_get_api): + dfw_rule1 = {'id': 'rule_id1', 'action': 'ALLOW', + 'display_name': 'rule1', 'description': None, + 'direction': 'IN_OUT', 'ip_protocol': 'IPV4_IPV6', + 'logged': False, + 'destination_groups': ['destination_url'], + 'source_groups': ['src_url'], 'resource_type': 'Rule', + 'scope': None, 'sequence_number': None, 'tag': None, + 'services': ['ANY'], "_create_time": 1} + dfw_rule2 = {'id': 'rule_id2', 'action': 'DROP', + 'display_name': 'rule2', 'description': None, + 'direction': 'IN_OUT', 'ip_protocol': 'IPV4_IPV6', + 'logged': False, + 'destination_groups': ['destination_url'], + 'source_groups': ['src_url'], 'resource_type': 'Rule', + 'scope': None, 'sequence_number': None, 'tag': None, + 'services': ['ANY'], "_create_time": 1} + security_policy = {'id': 'security_policy_id1', + 'display_name': 'security_policy', + 'category': 'Application', + 'resource_type': 'SecurityPolicy'} + domain = {'resource_type': 'Domain', 'id': 'domain1'} + domain_id = domain['id'] + map_id = security_policy['id'] + new_rule_name = 'new_rule1' + new_direction = 'IN' + dfw_rule_entries = [self.policy_lib.comm_map.build_entry( + name=new_rule_name, + domain_id=domain_id, + map_id=map_id, + entry_id=dfw_rule1['id'], + source_groups=dfw_rule1['source_groups'], + dest_groups=dfw_rule1['destination_groups'], + direction=new_direction + )] + + def get_group_path(group_id, domain_id): + return '/infra/domains/' + domain_id + '/groups/' + group_id + + for dfw_rule in [dfw_rule1, dfw_rule2]: + dfw_rule['destination_groups'] = [get_group_path(group_id, + domain_id) + for group_id in + dfw_rule['destination_groups']] + dfw_rule['source_groups'] = [get_group_path(group_id, domain_id) + for group_id in + dfw_rule['source_groups']] + + security_policy_values = copy.deepcopy(security_policy) + security_policy_values.update({'rules': + copy.deepcopy([dfw_rule1, dfw_rule2])}) + mock_get_api.return_value = security_policy_values + + with trans.NsxPolicyTransaction(): + self.policy_lib.comm_map.update_with_entries( + name=security_policy['display_name'], + domain_id=domain_id, + map_id=map_id, + entries=dfw_rule_entries + ) + + dfw_rule1['display_name'] = new_rule_name + dfw_rule1['direction'] = new_direction + child_rules = [{'resource_type': 'ChildRule', 'Rule': dfw_rule1}, + {'resource_type': 'ChildRule', 'Rule': dfw_rule2, + 'marked_for_delete': True}] + security_policy.update({'children': child_rules}) + child_security_policies = [{ + 'resource_type': 'ChildSecurityPolicy', + 'SecurityPolicy': security_policy + }] + domain.update({'children': child_security_policies}) + child_domains = [{ + 'resource_type': 'ChildDomain', + 'Domain': domain + }] + expected_body = {'resource_type': 'Infra', + 'children': child_domains} + self.assert_infra_patch_call(expected_body) + + @mock.patch('vmware_nsxlib.v3.policy.core_defs.NsxPolicyApi.get') + def test_updating_security_policy_with_no_entries_set(self, mock_get_api): + dfw_rule1 = {'id': 'rule_id1', 'action': 'ALLOW', + 'display_name': 'rule1', 'description': None, + 'direction': 'IN_OUT', 'ip_protocol': 'IPV4_IPV6', + 'logged': False, + 'destination_groups': ['destination_url'], + 'source_groups': ['src_url'], 'resource_type': 'Rule', + 'scope': None, 'sequence_number': None, 'tag': None, + 'services': ['ANY'], "_create_time": 1} + security_policy = {'id': 'security_policy_id1', + 'display_name': 'security_policy', + 'category': 'Application', + 'resource_type': 'SecurityPolicy'} + domain = {'resource_type': 'Domain', 'id': 'domain1'} + domain_id = domain['id'] + map_id = security_policy['id'] + + def get_group_path(group_id, domain_id): + return '/infra/domains/' + domain_id + '/groups/' + group_id + + for dfw_rule in [dfw_rule1]: + dfw_rule['destination_groups'] = [get_group_path(group_id, + domain_id) + for group_id in + dfw_rule['destination_groups']] + dfw_rule['source_groups'] = [get_group_path(group_id, domain_id) + for group_id in + dfw_rule['source_groups']] + + security_policy.update({'rules': [dfw_rule1]}) + mock_get_api.return_value = security_policy + + with trans.NsxPolicyTransaction(): + self.policy_lib.comm_map.update_with_entries( + name=security_policy['display_name'], + domain_id=domain_id, + map_id=map_id + ) + + child_security_policies = [{ + 'resource_type': 'ChildSecurityPolicy', + 'SecurityPolicy': security_policy + }] + domain.update({'children': child_security_policies}) + child_domains = [{ + 'resource_type': 'ChildDomain', + 'Domain': domain + }] + expected_body = {'resource_type': 'Infra', + 'children': child_domains} + self.assert_infra_patch_call(expected_body) diff --git a/vmware_nsxlib/v3/policy/core_defs.py b/vmware_nsxlib/v3/policy/core_defs.py index 1846e7ee..bd073e2e 100644 --- a/vmware_nsxlib/v3/policy/core_defs.py +++ b/vmware_nsxlib/v3/policy/core_defs.py @@ -83,6 +83,9 @@ class ResourceDef(object): self.body = {} + # Whether this entry needs to be deleted + self.delete = False + # As of now, for some defs (ex: services) child entry is required, # meaning parent creation will fail without the child. # Unfortunately in transactional API policy still fails us, even if @@ -92,6 +95,12 @@ class ResourceDef(object): # TODO(annak): remove this if/when policy solves this self.mandatory_child_def = None + def set_delete(self): + self.delete = True + + def get_delete(self): + return self.delete + def get_obj_dict(self): body = self.body if self.body else {} if self.resource_type(): @@ -1481,6 +1490,17 @@ class SecurityPolicyRuleBaseDef(ResourceDef): body['services'] = self.get_services_path(service_ids) return body + @classmethod + def adapt_from_rule_dict(cls, rule_dict, domain_id, map_id): + entry_id = rule_dict.pop('id', None) + name = rule_dict.pop('display_name', None) + + rule_def = cls(tenant=constants.POLICY_INFRA_TENANT, + domain_id=domain_id, map_id=map_id, entry_id=entry_id, + name=name) + rule_def.set_obj_dict(rule_dict) + return rule_def + class CommunicationMapEntryDef(SecurityPolicyRuleBaseDef): diff --git a/vmware_nsxlib/v3/policy/core_resources.py b/vmware_nsxlib/v3/policy/core_resources.py index b25ddb09..21d0ef91 100644 --- a/vmware_nsxlib/v3/policy/core_resources.py +++ b/vmware_nsxlib/v3/policy/core_resources.py @@ -3031,7 +3031,7 @@ class NsxPolicySecurityPolicyBaseApi(NsxPolicyResourceBase): delay=self.nsxlib_config.realization_wait_sec, max_attempts=self.nsxlib_config.realization_max_attempts) def _do_create_with_retry(): - self.policy_api.create_with_parent(map_def, entries) + self._create_or_store(map_def, entries) _do_create_with_retry() return map_id @@ -3219,34 +3219,63 @@ class NsxPolicySecurityPolicyBaseApi(NsxPolicyResourceBase): map_sequence_number=map_sequence_number) map_path = map_def.get_resource_path() - def _overwrite_entries(old_entries, new_entries): + def _overwrite_entries(old_entries, new_entries, transaction): # Replace old entries with new entries, but copy additional - # attributes from old entries for those kept in new entries. + # attributes from old entries for those kept in new entries + # and marked the unwanted ones in the old entries as deleted + # if it is in the transaction call. old_rules = {entry["id"]: entry for entry in old_entries} - new_rules = [] + replaced_entries = [] for entry in new_entries: rule_id = entry.get_id() new_rule = entry.get_obj_dict() old_rule = old_rules.get(rule_id) if old_rule: + old_rules.pop(rule_id) for key, value in old_rule.items(): if key not in new_rule: new_rule[key] = value - new_rules.append(new_rule) - return new_rules + replaced_entries.append( + self.entry_def.adapt_from_rule_dict( + new_rule, domain_id, map_id)) + + if transaction: + replaced_entries.extend( + _mark_delete_entries(old_rules.values())) + return replaced_entries + + def _mark_delete_entries(delete_rule_dicts): + delete_entries = [] + for delete_rule_dict in delete_rule_dicts: + delete_entry = self.entry_def.adapt_from_rule_dict( + delete_rule_dict, domain_id, map_id) + delete_entry.set_delete() + delete_entries.append(delete_entry) + return delete_entries @utils.retry_upon_exception( exceptions.StaleRevision, max_attempts=self.policy_api.client.max_attempts) def _update(): + transaction = trans.NsxPolicyTransaction.get_current() # Get the current data of communication map & its entries comm_map = self.policy_api.get(map_def) + replaced_entries = None + ignore_entries = (entries == IGNORE) + if not ignore_entries: + replaced_entries = _overwrite_entries(comm_map['rules'], + entries, transaction) + comm_map.pop('rules') map_def.set_obj_dict(comm_map) - body = map_def.get_obj_dict() - if entries != IGNORE: - body['rules'] = _overwrite_entries(comm_map['rules'], entries) # Update the entire map at the NSX - self.policy_api.client.update(map_path, body) + if transaction: + self._create_or_store(map_def, replaced_entries) + else: + body = map_def.get_obj_dict() + if not ignore_entries: + body['rules'] = [rule.get_obj_dict() for rule in + replaced_entries] + self.policy_api.client.update(map_path, body) _update() diff --git a/vmware_nsxlib/v3/policy/transaction.py b/vmware_nsxlib/v3/policy/transaction.py index 0599d189..03bc6157 100644 --- a/vmware_nsxlib/v3/policy/transaction.py +++ b/vmware_nsxlib/v3/policy/transaction.py @@ -67,7 +67,10 @@ class NsxPolicyTransaction(object): self.client = client # TODO(annak): raise exception for different tenants - self.defs.append(resource_def) + if isinstance(resource_def, list): + self.defs.extend(resource_def) + else: + self.defs.append(resource_def) def _sort_defs(self): sorted_defs = [] @@ -96,9 +99,12 @@ class NsxPolicyTransaction(object): self.defs = sorted_defs - def _build_wrapper_dict(self, resource_class, node): - return {'resource_type': 'Child%s' % resource_class, - resource_class: node} + def _build_wrapper_dict(self, resource_class, node, delete=False): + wrapper_dict = {'resource_type': 'Child%s' % resource_class, + resource_class: node} + if delete: + wrapper_dict.update({'marked_for_delete': True}) + return wrapper_dict def _find_parent_in_dict(self, d, resource_def, level=1): @@ -179,8 +185,9 @@ class NsxPolicyTransaction(object): child_dict_key = child_def.get_last_section_dict_key node[child_dict_key] = [child_def.get_obj_dict()] parent_dict['children'].append( - self._build_wrapper_dict(resource_class, node)) - + self._build_wrapper_dict(resource_class, + node, + resource_def.get_delete())) if body: headers = {'nsx-enable-partial-patch': 'true'} self.client.patch(url, body, headers=headers)