diff --git a/vmware_nsx/nsxlib/v3/dfw_api.py b/vmware_nsx/nsxlib/v3/dfw_api.py index 9731cdae0c..9305caecac 100644 --- a/vmware_nsx/nsxlib/v3/dfw_api.py +++ b/vmware_nsx/nsxlib/v3/dfw_api.py @@ -37,11 +37,11 @@ ALLOW = 'ALLOW' DROP = 'DROP' REJECT = 'REJECT' -# filtering operators +# filtering operators and expressions EQUALS = 'EQUALS' +NSGROUP_SIMPLE_EXPRESSION = 'NSGroupSimpleExpression' NSGROUP = 'NSGroup' -NSGROUP_SIMPLE_EXPRESSION = 'NSGroupSimpleExpression' LOGICAL_SWITCH = 'LogicalSwitch' LOGICAL_PORT = 'LogicalPort' IPV4ADDRESS = 'IPv4Address' @@ -89,7 +89,8 @@ def create_nsgroup(display_name, description, tags): def list_nsgroups(): - return nsxclient.get_resource('ns-groups').get('results', []) + return nsxclient.get_resource( + 'ns-groups?populate_references=false').get('results', []) @utils.retry_upon_exception_nsxv3(nsx_exc.StaleRevision) @@ -100,18 +101,20 @@ def update_nsgroup(nsgroup_id, display_name, description): return nsxclient.update_resource('ns-groups/%s' % nsgroup_id, nsgroup) -@utils.retry_upon_exception_nsxv3(nsx_exc.StaleRevision) +def get_nsgroup_member_expression(target_type, target_id): + return {'resource_type': NSGROUP_SIMPLE_EXPRESSION, + 'target_property': 'id', + 'target_type': target_type, + 'op': EQUALS, + 'value': target_id} + + def add_nsgroup_member(nsgroup_id, target_type, target_id): - nsgroup = read_nsgroup(nsgroup_id) - if 'members' not in nsgroup: - nsgroup['members'] = [] - nsgroup['members'].append({"resource_type": NSGROUP_SIMPLE_EXPRESSION, - 'target_property': 'id', - 'target_type': target_type, - 'op': EQUALS, - 'value': target_id}) + member_expr = get_nsgroup_member_expression(target_type, target_id) + data = {'members': [member_expr]} + add_members = 'ns-groups/%s?action=ADD_MEMBERS' % nsgroup_id try: - nsxclient.update_resource('ns-groups/%s' % nsgroup_id, nsgroup) + return nsxclient.create_resource(add_members, data) except nsx_exc.ManagerError: # REVISIT(roeyc): A ManagerError might have been raised for a # different reason, e.g - NSGroup does not exists. @@ -123,34 +126,27 @@ def add_nsgroup_member(nsgroup_id, target_type, target_id): raise NSGroupIsFull() -@utils.retry_upon_exception_nsxv3(nsx_exc.StaleRevision) -def remove_nsgroup_member(nsgroup_id, target_id, verify=False): - nsgroup = read_nsgroup(nsgroup_id) - for i, member in enumerate(nsgroup.get('members', [])): - if target_id == member['value']: - break - else: +def remove_nsgroup_member(nsgroup_id, target_type, target_id, verify=False): + member_expr = get_nsgroup_member_expression(target_type, target_id) + data = {'members': [member_expr]} + remove_members = 'ns-groups/%s?action=REMOVE_MEMBERS' % nsgroup_id + try: + return nsxclient.create_resource(remove_members, data) + except nsx_exc.ManagerError: if verify: err_msg = _("Failed to remove member %(tid)s " "from NSGroup %(nid)s.") % {'tid': target_id, 'nid': nsgroup_id} raise NSGroupMemberNotFound(err_msg=err_msg) - return - del nsgroup['members'][i] - return nsxclient.update_resource('ns-groups/%s' % nsgroup_id, nsgroup) def read_nsgroup(nsgroup_id): - nsgroup = nsxclient.get_resource('ns-groups/%s' % nsgroup_id) - # NSX API will not specify the 'members' attribute if its empty, but - # requires it on update. - if 'members' not in nsgroup: - nsgroup['members'] = [] - return nsgroup + return nsxclient.get_resource( + 'ns-groups/%s?populate_references=true' % nsgroup_id) def delete_nsgroup(nsgroup_id): - return nsxclient.delete_resource('ns-groups/%s' % nsgroup_id) + return nsxclient.delete_resource('ns-groups/%s?force=true' % nsgroup_id) def _build_section(display_name, description, applied_tos, tags): diff --git a/vmware_nsx/nsxlib/v3/security.py b/vmware_nsx/nsxlib/v3/security.py index 8ba6bb4bf4..6f2b0dbd5b 100644 --- a/vmware_nsx/nsxlib/v3/security.py +++ b/vmware_nsx/nsxlib/v3/security.py @@ -201,7 +201,7 @@ def update_lport_with_security_groups(context, lport_id, original, updated): for sg_id in removed: nsgroup_id, s = get_sg_mappings(context.session, sg_id) firewall.remove_nsgroup_member( - nsgroup_id, lport_id) + nsgroup_id, firewall.LOGICAL_PORT, lport_id) def init_nsgroup_manager_and_default_section_rules(): @@ -357,7 +357,7 @@ class NSGroupManager(object): for group in self._suggest_nested_group(nsgroup_id): try: firewall.remove_nsgroup_member( - group, nsgroup_id, verify=True) + group, firewall.NSGROUP, nsgroup_id, verify=True) break except firewall.NSGroupMemberNotFound: LOG.warning(_LW("NSGroup %(nsgroup)s was expected to be found " diff --git a/vmware_nsx/plugins/nsx_v3/plugin.py b/vmware_nsx/plugins/nsx_v3/plugin.py index 8b655e6207..40dd95269d 100644 --- a/vmware_nsx/plugins/nsx_v3/plugin.py +++ b/vmware_nsx/plugins/nsx_v3/plugin.py @@ -737,6 +737,8 @@ class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin, _net_id, nsx_port_id = nsx_db.get_nsx_switch_and_port_id( context.session, port_id) self._port_client.delete(nsx_port_id) + security.update_lport_with_security_groups( + context, nsx_port_id, port.get(ext_sg.SECURITYGROUPS, []), []) self.disassociate_floatingips(context, port_id) nsx_rpc.handle_port_metadata_access(self, context, port, is_delete=True) @@ -1520,8 +1522,8 @@ class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin, nsgroup_id, section_id = security.get_sg_mappings(context.session, id) super(NsxV3Plugin, self).delete_security_group(context, id) firewall.delete_section(section_id) - self.nsgroup_manager.remove_nsgroup(nsgroup_id) firewall.delete_nsgroup(nsgroup_id) + self.nsgroup_manager.remove_nsgroup(nsgroup_id) def create_security_group_rule(self, context, security_group_rule): bulk_rule = {'security_group_rules': [security_group_rule]} diff --git a/vmware_nsx/tests/unit/extensions/test_securitygroup.py b/vmware_nsx/tests/unit/extensions/test_securitygroup.py index 34a833d8ef..644d43036c 100644 --- a/vmware_nsx/tests/unit/extensions/test_securitygroup.py +++ b/vmware_nsx/tests/unit/extensions/test_securitygroup.py @@ -85,7 +85,8 @@ class TestSecurityGroups(test_nsxv3.NsxV3PluginTestCaseMixin, mock.call(NSG_IDS[2], mock.ANY, mock.ANY)] add_member_mock.assert_has_calls(calls, any_order=True) - remove_member_mock.assert_called_with(NSG_IDS[0], mock.ANY) + remove_member_mock.assert_called_with( + NSG_IDS[0], firewall.LOGICAL_PORT, mock.ANY) @_mock_create_and_list_nsgroups @mock.patch.object(firewall, 'remove_nsgroup_member') @@ -96,8 +97,10 @@ class TestSecurityGroups(test_nsxv3.NsxV3PluginTestCaseMixin, super(TestSecurityGroups, self).test_update_port_remove_security_group_empty_list() - add_member_mock.assert_called_with(NSG_IDS[1], mock.ANY, mock.ANY) - remove_member_mock.assert_called_with(NSG_IDS[1], mock.ANY) + add_member_mock.assert_called_with( + NSG_IDS[1], firewall.LOGICAL_PORT, mock.ANY) + remove_member_mock.assert_called_with( + NSG_IDS[1], firewall.LOGICAL_PORT, mock.ANY) class TestNSGroupManager(nsxlib_testcase.NsxLibTestCase): @@ -153,7 +156,7 @@ class TestNSGroupManager(nsxlib_testcase.NsxLibTestCase): add_member_mock.assert_called_once_with( NSG_IDS[2], firewall.NSGROUP, nsgroup_id) remove_member_mock.assert_called_once_with( - NSG_IDS[2], nsgroup_id, verify=True) + NSG_IDS[2], firewall.NSGROUP, nsgroup_id, verify=True) @_mock_create_and_list_nsgroups @mock.patch.object(firewall, 'remove_nsgroup_member') @@ -166,7 +169,7 @@ class TestNSGroupManager(nsxlib_testcase.NsxLibTestCase): if nsgroup == NSG_IDS[2]: raise firewall.NSGroupIsFull() - def _remove_member_mock(nsgroup, target_id, verify=False): + def _remove_member_mock(nsgroup, target_type, target_id, verify=False): if nsgroup == NSG_IDS[2]: raise firewall.NSGroupMemberNotFound() @@ -191,8 +194,11 @@ class TestNSGroupManager(nsxlib_testcase.NsxLibTestCase): # Since the nsgroup was added to the nested group at index 3, it will # fail to remove it from the group at index 2, and then will try to # remove it from the group at index 3. - calls = [mock.call(NSG_IDS[2], nsgroup_id, verify=True), - mock.call(NSG_IDS[3], nsgroup_id, verify=True)] + calls = [ + mock.call( + NSG_IDS[2], firewall.NSGROUP, nsgroup_id, verify=True), + mock.call( + NSG_IDS[3], firewall.NSGROUP, nsgroup_id, verify=True)] remove_member_mock.assert_has_calls(calls) @_mock_create_and_list_nsgroups