From fb26288ad7a15e3da159ddc6d8993f6e7d2510b2 Mon Sep 17 00:00:00 2001 From: asarfaty Date: Sun, 8 Dec 2019 14:11:31 +0200 Subject: [PATCH] NSX|P: Change order of NAT and Edge Firewall rules Adding a new configuration to let the admin control if the edge firewall rule will see the external addresses or internal ones, thus controlling the order of implementation. The new parameter firewall_match_internal_addr is True by default so it is backwards compatible. In addition, adding an admin utility to change this flag across all existing nat rules. Depends-on: Ia34e42a94c10bd3f12ebc658939ed826af53658c Change-Id: I29e7acc03bf6b845d9a727cf075cbe2b0609af34 --- doc/source/admin_util.rst | 3 ++ vmware_nsx/common/config.py | 5 +++ vmware_nsx/plugins/nsx_p/plugin.py | 13 ++++-- .../admin/plugins/nsxp/resources/routers.py | 45 +++++++++++++++++++ vmware_nsx/shell/resources.py | 4 +- vmware_nsx/tests/unit/nsx_p/test_plugin.py | 34 +++++++++++--- 6 files changed, 95 insertions(+), 9 deletions(-) diff --git a/doc/source/admin_util.rst b/doc/source/admin_util.rst index f2f642ad8b..a2cb516726 100644 --- a/doc/source/admin_util.rst +++ b/doc/source/admin_util.rst @@ -638,6 +638,9 @@ NSX Policy Plugin nsxadmin -r routers -o update-tier0 --property old-tier0= --property new-tier0= +- Update the firewall_match value in neutron nat rules with a new value. Should be used when firewall_match_internal_addr config changes:: + + nsxadmin -r routers -o update-nat-firewall-match --property firewall-match=external/internal Client Certificate ~~~~~~~~~~~~~~~~~~ diff --git a/vmware_nsx/common/config.py b/vmware_nsx/common/config.py index bb4eed131a..da5efec355 100644 --- a/vmware_nsx/common/config.py +++ b/vmware_nsx/common/config.py @@ -542,6 +542,11 @@ nsx_p_opts = nsx_v3_and_p + [ default=1.0, help=_("(Optional) Number of seconds to wait between attempts " "for a resource to be realized")), + cfg.BoolOpt('firewall_match_internal_addr', + default=True, + help=_("If True, edge firewall rules will match internal " + "addresses. Else they will match the external " + "addresses")), ] diff --git a/vmware_nsx/plugins/nsx_p/plugin.py b/vmware_nsx/plugins/nsx_p/plugin.py index 8e5aacab72..0523312be9 100644 --- a/vmware_nsx/plugins/nsx_p/plugin.py +++ b/vmware_nsx/plugins/nsx_p/plugin.py @@ -1493,6 +1493,7 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): gw_address_scope): return + firewall_match = self._get_nat_firewall_match() self.nsxpolicy.tier1_nat_rule.create_or_overwrite( 'snat for subnet %s' % subnet['id'], router_id, @@ -1501,7 +1502,7 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): sequence_number=NAT_RULE_PRIORITY_GW, translated_network=gw_ip, source_network=subnet['cidr'], - firewall_match=policy_constants.NAT_FIREWALL_MATCH_INTERNAL) + firewall_match=firewall_match) def _get_snat_rule_id(self, subnet): return 'S-' + subnet['id'] @@ -2202,7 +2203,13 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): def _get_fip_dnat_rule_id(self, fip_id): return 'D-' + fip_id + def _get_nat_firewall_match(self): + if cfg.CONF.nsx_p.firewall_match_internal_addr: + return policy_constants.NAT_FIREWALL_MATCH_INTERNAL + return policy_constants.NAT_FIREWALL_MATCH_EXTERNAL + def _add_fip_nat_rules(self, tier1_id, fip_id, ext_ip, int_ip): + firewall_match = self._get_nat_firewall_match() with policy_trans.NsxPolicyTransaction(): self.nsxpolicy.tier1_nat_rule.create_or_overwrite( 'snat for fip %s' % fip_id, @@ -2212,7 +2219,7 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): translated_network=ext_ip, source_network=int_ip, sequence_number=NAT_RULE_PRIORITY_FIP, - firewall_match=policy_constants.NAT_FIREWALL_MATCH_INTERNAL) + firewall_match=firewall_match) self.nsxpolicy.tier1_nat_rule.create_or_overwrite( 'dnat for fip %s' % fip_id, tier1_id, @@ -2221,7 +2228,7 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): translated_network=int_ip, destination_network=ext_ip, sequence_number=NAT_RULE_PRIORITY_FIP, - firewall_match=policy_constants.NAT_FIREWALL_MATCH_INTERNAL) + firewall_match=firewall_match) def _delete_fip_nat_rules(self, tier1_id, fip_id): with policy_trans.NsxPolicyTransaction(): diff --git a/vmware_nsx/shell/admin/plugins/nsxp/resources/routers.py b/vmware_nsx/shell/admin/plugins/nsxp/resources/routers.py index ce68e7462e..77fb7a0741 100644 --- a/vmware_nsx/shell/admin/plugins/nsxp/resources/routers.py +++ b/vmware_nsx/shell/admin/plugins/nsxp/resources/routers.py @@ -24,6 +24,9 @@ from vmware_nsx.shell.admin.plugins.common import utils as admin_utils from vmware_nsx.shell.admin.plugins.nsxp.resources import utils as p_utils from vmware_nsx.shell import resources as shell +from vmware_nsxlib.v3.policy import constants as policy_constants +from vmware_nsxlib.v3.policy import transaction as policy_trans + LOG = logging.getLogger(__name__) @@ -114,6 +117,48 @@ def update_tier0(resource, event, trigger, **kwargs): LOG.info("Done.") +@admin_utils.output_header +def update_nat_firewall_match(resource, event, trigger, **kwargs): + """Update the firewall_match value in neutron nat rules with a new value""" + errmsg = ("Need to specify internal/external firewall_match value. " + "Add --property firewall-match=") + if not kwargs.get('property'): + LOG.error("%s", errmsg) + return + properties = admin_utils.parse_multi_keyval_opt(kwargs['property']) + firewall_match_str = properties.get('firewall-match') + if (not firewall_match_str or + firewall_match_str.lower() not in ('internal', 'external')): + LOG.error("%s", errmsg) + return + + if firewall_match_str.lower() == 'internal': + new_firewall_match = policy_constants.NAT_FIREWALL_MATCH_INTERNAL + old_firewall_match = policy_constants.NAT_FIREWALL_MATCH_EXTERNAL + else: + new_firewall_match = policy_constants.NAT_FIREWALL_MATCH_EXTERNAL + old_firewall_match = policy_constants.NAT_FIREWALL_MATCH_INTERNAL + + nsxpolicy = p_utils.get_connected_nsxpolicy() + plugin = RoutersPlugin() + ctx = context.get_admin_context() + neutron_routers = plugin.get_routers(ctx) + for router in neutron_routers: + rules = nsxpolicy.tier1_nat_rule.list(router['id']) + for rule in rules: + with policy_trans.NsxPolicyTransaction(): + if rule['firewall_match'] == old_firewall_match: + nsxpolicy.tier1_nat_rule.update( + router['id'], rule['id'], + firewall_match=new_firewall_match) + + LOG.info("Done.") + + registry.subscribe(update_tier0, constants.ROUTERS, shell.Operations.UPDATE_TIER0.value) + +registry.subscribe(update_nat_firewall_match, + constants.ROUTERS, + shell.Operations.UPDATE_FIREWALL_MATCH.value) diff --git a/vmware_nsx/shell/resources.py b/vmware_nsx/shell/resources.py index b80790d2dd..99391a51ce 100644 --- a/vmware_nsx/shell/resources.py +++ b/vmware_nsx/shell/resources.py @@ -72,6 +72,7 @@ class Operations(enum.Enum): VALIDATE = 'validate' REUSE = 'reuse' UPDATE_TIER0 = 'update-tier0' + UPDATE_FIREWALL_MATCH = 'update-nat-firewall-match' ops = [op.value for op in Operations] @@ -262,7 +263,8 @@ nsxp_resources = { [Operations.LIST.value]), constants.ROUTERS: Resource(constants.ROUTERS, [Operations.LIST.value, - Operations.UPDATE_TIER0.value]), + Operations.UPDATE_TIER0.value, + Operations.UPDATE_FIREWALL_MATCH.value]), constants.CERTIFICATE: Resource(constants.CERTIFICATE, [Operations.GENERATE.value, Operations.SHOW.value, diff --git a/vmware_nsx/tests/unit/nsx_p/test_plugin.py b/vmware_nsx/tests/unit/nsx_p/test_plugin.py index 4b7b40b217..9310d06953 100644 --- a/vmware_nsx/tests/unit/nsx_p/test_plugin.py +++ b/vmware_nsx/tests/unit/nsx_p/test_plugin.py @@ -56,7 +56,7 @@ from vmware_nsx.tests import unit as vmware from vmware_nsx.tests.unit.common_plugin import common_v3 from vmware_nsxlib.v3 import exceptions as nsxlib_exc from vmware_nsxlib.v3 import nsx_constants -from vmware_nsxlib.v3.policy import constants as policy_constants +from vmware_nsxlib.v3.policy import constants as pol_const from vmware_nsxlib.v3 import utils as nsxlib_utils @@ -117,7 +117,7 @@ class NsxPPluginTestCaseMixin( return_value=-1).start() mock.patch("vmware_nsxlib.v3.policy.core_resources." "NsxPolicyResourceBase._wait_until_realized", - return_value={'state': policy_constants.STATE_REALIZED} + return_value={'state': pol_const.STATE_REALIZED} ).start() mock.patch("vmware_nsxlib.v3.policy.core_resources." "NsxPolicyTier1Api.update_transport_zone").start() @@ -1218,14 +1218,14 @@ class NsxPTestSecurityGroup(common_v3.FixExternalNetBaseTest, sg_id = sg['security_group']['id'] nsx_name = utils.get_name_and_uuid(name, sg_id) group_create.assert_called_once_with( - nsx_name, policy_constants.DEFAULT_DOMAIN, group_id=sg_id, + nsx_name, pol_const.DEFAULT_DOMAIN, group_id=sg_id, description=description, conditions=[mock.ANY], tags=mock.ANY) comm_map_create.assert_called_once_with( - nsx_name, policy_constants.DEFAULT_DOMAIN, map_id=sg_id, + nsx_name, pol_const.DEFAULT_DOMAIN, map_id=sg_id, description=description, tags=mock.ANY, - category=policy_constants.CATEGORY_ENVIRONMENT) + category=pol_const.CATEGORY_ENVIRONMENT) def _create_provider_security_group(self): body = {'security_group': {'name': 'provider-deny', @@ -2296,3 +2296,27 @@ class NsxPTestL3NatTestCase(NsxPTestL3NatTest, self._delete('routers', r['router']['id']) routers = self.plugin.get_routers(self.ctx) self.assertEqual(0, len(routers)) + + def _test_nat_rules_firewall_match(self, config_val, call_val): + cfg.CONF.set_override('firewall_match_internal_addr', config_val, + 'nsx_p') + with self.subnet(cidr='20.0.0.0/24') as subnet: + self._set_net_external(subnet['subnet']['network_id']) + with mock.patch("vmware_nsxlib.v3.policy.core_resources." + "NsxPolicyTier1NatRuleApi." + "create_or_overwrite") as add_func,\ + self.floatingip_with_assoc(): + add_func.assert_called_with( + mock.ANY, mock.ANY, action='DNAT', + destination_network=mock.ANY, + firewall_match=call_val, + nat_rule_id=mock.ANY, sequence_number=mock.ANY, + translated_network=mock.ANY) + + def test_nat_rules_firewall_match_internal(self): + self._test_nat_rules_firewall_match( + True, pol_const.NAT_FIREWALL_MATCH_INTERNAL) + + def test_nat_rules_firewall_match_external(self): + self._test_nat_rules_firewall_match( + False, pol_const.NAT_FIREWALL_MATCH_EXTERNAL)