x/vmware-nsx
Code Issues Proposed changes

NSX|P: Use transactions for security groups and rules creation

Browse Source 
Change-Id: I470b2a349fb46c7af3e450db240d9cdca2f6b70c
This commit is contained in:
asarfaty 2019-11-13 12:09:04 +02:00 committed by Adit Sarfaty
parent e0cff006c3
commit eab7958d8a
3 changed files with 64 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

116
vmware_nsx/plugins/nsx_p/plugin.py
View File

@ -2544,38 +2544,29 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
cond_val=scope_and_tag, cond_val=scope_and_tag,
cond_key=policy_constants.CONDITION_KEY_TAG, cond_key=policy_constants.CONDITION_KEY_TAG,
cond_member_type=policy_constants.CONDITION_MEMBER_PORT) cond_member_type=policy_constants.CONDITION_MEMBER_PORT)
# Create the group
try:
self.nsxpolicy.group.create_or_overwrite_with_conditions(
nsx_name, NSX_P_GLOBAL_DOMAIN_ID, group_id=sg_id,
description=secgroup.get('description'),
conditions=[condition], tags=tags)
except Exception as e:
msg = (_("Failed to create NSX group for SG %(sg)s: "
"%(e)s") % {'sg': sg_id, 'e': e})
raise nsx_exc.NsxPluginException(err_msg=msg)
category = NSX_P_REGULAR_SECTION_CATEGORY category = NSX_P_REGULAR_SECTION_CATEGORY
if secgroup.get(provider_sg.PROVIDER) is True: if secgroup.get(provider_sg.PROVIDER) is True:
category = NSX_P_PROVIDER_SECTION_CATEGORY category = NSX_P_PROVIDER_SECTION_CATEGORY
# create the communication map (=section) and entries (=rules)
try: try:
if entries: with policy_trans.NsxPolicyTransaction():
self.nsxpolicy.comm_map.create_with_entries( # Create the group
nsx_name, NSX_P_GLOBAL_DOMAIN_ID, map_id=sg_id, self.nsxpolicy.group.create_or_overwrite_with_conditions(
nsx_name, NSX_P_GLOBAL_DOMAIN_ID, group_id=sg_id,
description=secgroup.get('description'), description=secgroup.get('description'),
entries=entries, conditions=[condition], tags=tags)
tags=tags, category=category)
else: # create the communication map (=section) and entries (=rules)
self.nsxpolicy.comm_map.create_or_overwrite_map_only( self.nsxpolicy.comm_map.create_or_overwrite_map_only(
nsx_name, NSX_P_GLOBAL_DOMAIN_ID, map_id=sg_id, nsx_name, NSX_P_GLOBAL_DOMAIN_ID, map_id=sg_id,
description=secgroup.get('description'), description=secgroup.get('description'),
tags=tags, category=category) tags=tags, category=category)
for entry in entries:
self.nsxpolicy.comm_map.create_entry_from_def(entry)
except Exception as e: except Exception as e:
msg = (_("Failed to create NSX communication map for SG %(sg)s: " msg = (_("Failed to create NSX resources for SG %(sg)s: "
"%(e)s") % {'sg': sg_id, 'e': e}) "%(e)s") % {'sg': sg_id, 'e': e})
self.nsxpolicy.group.delete(NSX_P_GLOBAL_DOMAIN_ID, sg_id)
raise nsx_exc.NsxPluginException(err_msg=msg) raise nsx_exc.NsxPluginException(err_msg=msg)
def _get_rule_ip_protocol(self, sg_rule): def _get_rule_ip_protocol(self, sg_rule):
@ -2643,8 +2634,7 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
def _create_security_group_backend_rule(self, context, map_id, def _create_security_group_backend_rule(self, context, map_id,
sg_rule, secgroup_logging, sg_rule, secgroup_logging,
is_provider_sg=False, is_provider_sg=False):
create_rule=True):
"""Create backend resources for a DFW rule """Create backend resources for a DFW rule
All rule resources (service, groups) will be created All rule resources (service, groups) will be created
@ -2706,34 +2696,20 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
this_group_id)] this_group_id)]
action = (policy_constants.ACTION_DENY if is_provider_sg action = (policy_constants.ACTION_DENY if is_provider_sg
else policy_constants.ACTION_ALLOW) else policy_constants.ACTION_ALLOW)
if create_rule: # Just return the rule entry without creating it
self.nsxpolicy.comm_map.create_entry( rule_entry = self.nsxpolicy.comm_map.build_entry(
nsx_name, NSX_P_GLOBAL_DOMAIN_ID, nsx_name, NSX_P_GLOBAL_DOMAIN_ID,
map_id, entry_id=sg_rule['id'], map_id, entry_id=sg_rule['id'],
description=sg_rule.get('description'), description=sg_rule.get('description'),
service_ids=[service] if service else None, service_ids=[service] if service else None,
ip_protocol=ip_protocol, ip_protocol=ip_protocol,
action=action, action=action,
source_groups=[source] if source else None, source_groups=[source] if source else None,
dest_groups=[destination] if destination else None, dest_groups=[destination] if destination else None,
scope=scope, scope=scope,
direction=direction, logged=logging, tag=sg_rule.get('project_id'),
tag=sg_rule.get('project_id')) direction=direction, logged=logging)
else: return rule_entry
# Just return the rule entry without creating it
rule_entry = self.nsxpolicy.comm_map.build_entry(
nsx_name, NSX_P_GLOBAL_DOMAIN_ID,
map_id, entry_id=sg_rule['id'],
description=sg_rule.get('description'),
service_ids=[service] if service else None,
ip_protocol=ip_protocol,
action=action,
source_groups=[source] if source else None,
dest_groups=[destination] if destination else None,
scope=scope,
tag=sg_rule.get('project_id'),
direction=direction, logged=logging)
return rule_entry
def create_security_group(self, context, security_group, default_sg=False): def create_security_group(self, context, security_group, default_sg=False):
secgroup = security_group['security_group'] secgroup = security_group['security_group']
@ -2768,11 +2744,13 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
sg_rules = secgroup_db['security_group_rules'] sg_rules = secgroup_db['security_group_rules']
secgroup_logging = secgroup.get(sg_logging.LOGGING, False) secgroup_logging = secgroup.get(sg_logging.LOGGING, False)
backend_rules = [] backend_rules = []
for sg_rule in sg_rules: with policy_trans.NsxPolicyTransaction():
rule_entry = self._create_security_group_backend_rule( # Create all the rules resources in a single transaction
context, secgroup_db['id'], sg_rule, for sg_rule in sg_rules:
secgroup_logging, create_rule=False) rule_entry = self._create_security_group_backend_rule(
backend_rules.append(rule_entry) context, secgroup_db['id'], sg_rule,
secgroup_logging)
backend_rules.append(rule_entry)
# Create Group & communication map on the NSX # Create Group & communication map on the NSX
self._create_security_group_backend_resources( self._create_security_group_backend_resources(
context, secgroup, backend_rules) context, secgroup, backend_rules)
@ -2885,13 +2863,29 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
is_provider_sg = sg.get(provider_sg.PROVIDER) is_provider_sg = sg.get(provider_sg.PROVIDER)
secgroup_logging = self._is_security_group_logged(context, sg_id) secgroup_logging = self._is_security_group_logged(context, sg_id)
for rule_data in rules_db: category = (NSX_P_PROVIDER_SECTION_CATEGORY if is_provider_sg
#TODO(asarfaty): Consider using update_entries with all the rules else NSX_P_REGULAR_SECTION_CATEGORY)
# if multiple rules are added # Create the NSX backend rules in a single transaction
# create the NSX backend rule with policy_trans.NsxPolicyTransaction():
self._create_security_group_backend_rule( # Build new rules and relevant objects
context, sg_id, rule_data, secgroup_logging, backend_rules = []
is_provider_sg=is_provider_sg) for rule_data in rules_db:
rule_entry = self._create_security_group_backend_rule(
context, sg_id, rule_data, secgroup_logging,
is_provider_sg=is_provider_sg)
backend_rules.append(rule_entry)
# Add the old rules
for rule in sg['security_group_rules']:
rule_entry = self.nsxpolicy.comm_map.build_entry(
NSX_P_GLOBAL_DOMAIN_ID, sg_id, rule['id'])
backend_rules.append(rule_entry)
# Update the policy with all the rules.
self.nsxpolicy.comm_map.update_with_entries(
NSX_P_GLOBAL_DOMAIN_ID, sg_id, entries=backend_rules,
category=category)
return rules_db return rules_db

26
vmware_nsx/tests/unit/extensions/test_provider_security_groups.py
View File

@ -28,8 +28,6 @@ from vmware_nsx.extensions import providersecuritygroup as provider_sg
from vmware_nsx.tests.unit.nsx_p import test_plugin as test_nsxp_plugin from vmware_nsx.tests.unit.nsx_p import test_plugin as test_nsxp_plugin
from vmware_nsx.tests.unit.nsx_v import test_plugin as test_nsxv_plugin from vmware_nsx.tests.unit.nsx_v import test_plugin as test_nsxv_plugin
from vmware_nsx.tests.unit.nsx_v3 import test_plugin as test_nsxv3_plugin from vmware_nsx.tests.unit.nsx_v3 import test_plugin as test_nsxv3_plugin
from vmware_nsxlib.v3 import nsx_constants
from vmware_nsxlib.v3.policy import constants as policy_constants
PLUGIN_NAME = ('vmware_nsx.tests.unit.extensions.' PLUGIN_NAME = ('vmware_nsx.tests.unit.extensions.'
@ -404,23 +402,7 @@ class TestNSXpProviderSecurityGrp(test_nsxp_plugin.NsxPPluginTestCaseMixin,
sg_id = provider_secgroup['security_group']['id'] sg_id = provider_secgroup['security_group']['id']
with mock.patch("vmware_nsxlib.v3.policy.core_resources." with mock.patch("vmware_nsxlib.v3.policy.core_resources."
"NsxPolicyCommunicationMapApi.create_entry" "NsxPolicyCommunicationMapApi.update_with_entries"
) as entry_create: ) as entry_create,\
with self.security_group_rule(security_group_id=sg_id) as rule: self.security_group_rule(security_group_id=sg_id):
rule_data = rule['security_group_rule'] entry_create.assert_called_once()
rule_id = rule_data['id']
scope = [self.plugin.nsxpolicy.group.get_path(
policy_constants.DEFAULT_DOMAIN, sg_id)]
entry_create.assert_called_once_with(
rule_id, policy_constants.DEFAULT_DOMAIN,
sg_id, entry_id=rule_id,
description='',
direction=nsx_constants.IN,
ip_protocol=nsx_constants.IPV4,
action=policy_constants.ACTION_DENY,
service_ids=mock.ANY,
source_groups=mock.ANY,
dest_groups=mock.ANY,
scope=scope,
logged=False,
tag=rule_data['project_id'])

26
vmware_nsx/tests/unit/nsx_p/test_plugin.py
View File

@ -1213,7 +1213,7 @@ class NsxPTestSecurityGroup(common_v3.FixExternalNetBaseTest,
) as group_create,\ ) as group_create,\
mock.patch("vmware_nsxlib.v3.policy.core_resources." mock.patch("vmware_nsxlib.v3.policy.core_resources."
"NsxPolicyCommunicationMapApi." "NsxPolicyCommunicationMapApi."
"create_with_entries") as comm_map_create,\ "create_or_overwrite_map_only") as comm_map_create,\
self.security_group(name, description) as sg: self.security_group(name, description) as sg:
sg_id = sg['security_group']['id'] sg_id = sg['security_group']['id']
nsx_name = utils.get_name_and_uuid(name, sg_id) nsx_name = utils.get_name_and_uuid(name, sg_id)
@ -1225,7 +1225,6 @@ class NsxPTestSecurityGroup(common_v3.FixExternalNetBaseTest,
nsx_name, policy_constants.DEFAULT_DOMAIN, map_id=sg_id, nsx_name, policy_constants.DEFAULT_DOMAIN, map_id=sg_id,
description=description, description=description,
tags=mock.ANY, tags=mock.ANY,
entries=mock.ANY,
category=policy_constants.CATEGORY_ENVIRONMENT) category=policy_constants.CATEGORY_ENVIRONMENT)
def _create_provider_security_group(self): def _create_provider_security_group(self):
@ -1299,28 +1298,13 @@ class NsxPTestSecurityGroup(common_v3.FixExternalNetBaseTest,
with self.security_group(name, description) as sg: with self.security_group(name, description) as sg:
sg_id = sg['security_group']['id'] sg_id = sg['security_group']['id']
with mock.patch("vmware_nsxlib.v3.policy.core_resources." with mock.patch("vmware_nsxlib.v3.policy.core_resources."
"NsxPolicyCommunicationMapApi.create_entry" "NsxPolicyCommunicationMapApi.update_with_entries"
) as entry_create,\ ) as update_policy,\
self.security_group_rule(sg_id, direction, self.security_group_rule(sg_id, direction,
protocol, port_range_min, protocol, port_range_min,
port_range_max, port_range_max,
remote_ip_prefix) as rule: remote_ip_prefix):
rule_id = rule['security_group_rule']['id'] update_policy.assert_called_once()
scope = [self.plugin.nsxpolicy.group.get_path(
policy_constants.DEFAULT_DOMAIN, sg_id)]
entry_create.assert_called_once_with(
rule_id, policy_constants.DEFAULT_DOMAIN,
sg_id, entry_id=rule_id,
description='',
direction=nsx_constants.IN,
ip_protocol=nsx_constants.IPV4,
action=policy_constants.ACTION_ALLOW,
service_ids=mock.ANY,
source_groups=mock.ANY,
dest_groups=mock.ANY,
scope=scope,
logged=False,
tag=mock.ANY)
def test_create_security_group_rule_with_remote_group(self): def test_create_security_group_rule_with_remote_group(self):
with self.security_group() as sg1, self.security_group() as sg2: with self.security_group() as sg1, self.security_group() as sg2: