From 72eac47b87459a317b5be99a841ba144fd83fbaa Mon Sep 17 00:00:00 2001 From: Sven Rosenzweig Date: Tue, 26 Mar 2024 12:38:22 +0100 Subject: [PATCH] Support updating default security policies Creating the default security policies (DHCP, ICMP and Metadata allow), happens on driver start (in case they have not been realized so far). Changing those was not supported. Before starting the driver, it is now checked if the realized rules differs from the rules defined in NSX-T constants. If a change is detected, the rules defined in the NSX-T constants will be realized as defined. --- .../nsxv3/agent/provider_nsx_policy.py | 38 +++- .../test_infrastrucutre_sg_rules.py | 193 ++++++++++++++++++ requirements.txt | 1 + 3 files changed, 231 insertions(+), 1 deletion(-) create mode 100644 networking_nsxv3/tests/unit/realization/test_infrastrucutre_sg_rules.py diff --git a/networking_nsxv3/plugins/ml2/drivers/nsxv3/agent/provider_nsx_policy.py b/networking_nsxv3/plugins/ml2/drivers/nsxv3/agent/provider_nsx_policy.py index a248516e..800141c1 100644 --- a/networking_nsxv3/plugins/ml2/drivers/nsxv3/agent/provider_nsx_policy.py +++ b/networking_nsxv3/plugins/ml2/drivers/nsxv3/agent/provider_nsx_policy.py @@ -10,6 +10,7 @@ import netaddr import functools from oslo_cache import core as cache +from dictdiffer import diff from oslo_config import cfg from oslo_log import log as logging from oslo_utils import excutils @@ -113,6 +114,9 @@ class API(object): INFRA = "/policy/api/v1/infra" +class Ignore(set): + def __contains__(self, key): + return set.__contains__(self, key[1]) class PolicyResourceMeta(base.ResourceMeta): def __init__(self, id, unique_id, rev, age, revision, last_modified_time, @@ -635,12 +639,44 @@ def _setup_default_infrastructure_rules(self): path = API.POLICY.format(policy["id"]) res = self.client.get(path=path) if res.ok: - continue + policy_changes = self._check_infrastructure_rules_for_updates(policy, res.json()) + if policy_changes: + LOG.info("Infrastructure Policy %s updated. Realize the following changes %s", + policy["display_name"], policy_changes) + policy_with_rev = self._add_revision_number(policy, res.json()) + self.client.put(path=path, data=policy_with_rev).raise_for_status() elif res.status_code == 404: LOG.info("Infrastructure Policy %s not found, creating...", policy["display_name"]) self.client.put(path=path, data=policy).raise_for_status() else: res.raise_for_status() + def _add_revision_number(self, policy_from_config, realized_policy): + """ + Add revision number for the sg policy and sg rule to the new policy + Updating NSX-T objects requires the revision number to be matched with the existing object + """ + revision = realized_policy.get("_revision", None) + + if revision is not None: + policy_from_config["_revision"] = revision + + realized_rules = sorted(realized_policy["rules"], key=lambda x: x['display_name']) + + for x, rule in enumerate(sorted(policy_from_config["rules"], key=lambda x: x['display_name'])): + _revision = realized_rules[x].get("_revision", None) + if revision is not None: + rule["_revision"] = _revision + return policy_from_config + + + def _check_infrastructure_rules_for_updates(self, policy_from_cfg, realized_policy): + rules_realized = sorted(realized_policy["rules"], key=lambda x: x['display_name']) + rules_from_config = sorted(policy_from_cfg['rules'], key=lambda x: x['display_name']) + + # Ignore all keys that are not in both lists + ignore_all = set(rules_realized[0]).symmetric_difference(set(rules_from_config[0])) + changes = list(diff(rules_realized, rules_from_config, ignore=Ignore(ignore_all))) + return changes def _setup_default_app_drop_logged_section(self): LOG.info("Looking for the Default Layer3 Logged Drop Section.") diff --git a/networking_nsxv3/tests/unit/realization/test_infrastrucutre_sg_rules.py b/networking_nsxv3/tests/unit/realization/test_infrastrucutre_sg_rules.py new file mode 100644 index 00000000..30b33d4d --- /dev/null +++ b/networking_nsxv3/tests/unit/realization/test_infrastrucutre_sg_rules.py @@ -0,0 +1,193 @@ +import copy + +import eventlet + +eventlet.monkey_patch() + +import responses +import json +import re + +from networking_nsxv3.common import config +from neutron.tests import base +from networking_nsxv3.plugins.ml2.drivers.nsxv3.agent.constants_nsx import DEFAULT_INFRASTRUCTURE_POLICIES +from networking_nsxv3.plugins.ml2.drivers.nsxv3.agent.provider_nsx_policy import Provider + + +class TestInfrastrucutreRuleChanges(base.BaseTestCase): + provider = None + + def setUp(self): + super().setUp() + with responses.RequestsMock(assert_all_requests_are_fired=False) as rsps: + rsps.add( + method=responses.GET, + url=re.compile(".*PolicyTransportZone.*"), + body='{"results": [{"display_name": "openstack-tz", "id": "openstack-tz", "tags": []}]}' + ) + rsps.add( + method=responses.GET, + url=re.compile('.*\/node\/version.*'), + body='{"product_version": "0.0.0"}' + ) + rsps.add( + method=responses.GET, + url=re.compile(".*default-layer3-logged-drop-section.*"), + status=200 + ) + rsps.add( + method=responses.GET, + url=re.compile(".*default-layer3-section.*"), + status=200, + body='{"rules": []}' + ) + self.provider = Provider() + + def tearDown(self): + super().tearDown() + self.provider = None + + def test_change_existing_rule(self): + """ + Tests whether changing an existing rule is detected + Additionally check if the _revision number is updated. + Updating objects in NSX-T requires the _revision number to be set. + """ + change_rule = { + "id": "ICMP_Allow", + "rules": [ + { + "action": "ALLOW", + "display_name": "ICMP", + "destination_groups": ["1.1.1.1", "8.8.8.8"] + } + ] + } + + realized_rule = { + "rules": [ + { + "action": "ALLOW", + "display_name": "ICMP", + "path": "/infra/domains/default/security-policies/ICMP_Allow/rules/ICMP", + "_revision": 1, + "destination_groups": [ + "1.1.1.1" + ], + } + ], + "logging_enabled": False, + "path": "/infra/domains/default/security-policies/ICMP_Allow", + "realization_id": "a579efb4-446e-47a1-a63a-ab88125a43f1", + "_revision": 0 + } + diff = Provider._check_infrastructure_rules_for_updates(None, change_rule, realized_rule) + with_revision = Provider._add_revision_number(None, change_rule, realized_rule) + self.assertEquals(list(diff), [ + ('add', [0, 'destination_groups'], [(1, '8.8.8.8')])]) + + self.assertEquals(with_revision["rules"][0]["_revision"], 1) + self.assertEquals(with_revision["_revision"], 0) + + def test_add_new_rule_to_policy(self): + """ + Tests whether adding a new rule to an existing policy is detected. + """ + change_rule = { + "id": "ICMP_Allow", + "rules": [ + { + "display_name": "ICMP", + }, + { + "display_name": "rule2", + } + ] + } + + realized_rule = { + "rules": [ + { + "display_name": "ICMP", + "_revision": 1 + } + ], + "_revision": 0 + } + diff = Provider._check_infrastructure_rules_for_updates(None, change_rule, realized_rule) + self.assertEquals(list(diff), + [('add', '', [(1, {'display_name': 'rule2'})])]) + + def _mock_fetch_infrastructure_policy(self, response): + response.add( + method=responses.GET, + url=re.compile(".*ICMP_Allow.*"), + body=json.dumps(DEFAULT_INFRASTRUCTURE_POLICIES[0]), + status=200 + ) + response.add( + method=responses.GET, + url=re.compile(".*Metadata_Allow.*"), + body=json.dumps(DEFAULT_INFRASTRUCTURE_POLICIES[1]), + status=200 + ) + response.add( + method=responses.GET, + url=re.compile(".*DHCP_Allow.*"), + body=json.dumps(DEFAULT_INFRASTRUCTURE_POLICIES[2]), + status=200 + ) + + def test_infrastructure_policy_present(self): + """ + Tests whether the infrastructure policies (Metadata Allow, ICMP Allow and DHCP Allow) are fetched from NSX-T. + No updates are made to the policies. + """ + with responses.RequestsMock() as rsps: + self._mock_fetch_infrastructure_policy(rsps) + + self.provider._setup_default_infrastructure_rules() + + #Request: GET ICMP_Allow, GET Metadata_Allow, GET DHCP_Allow + self.assertEquals(len(rsps.calls), 3) + self.assertEquals(rsps.assert_all_requests_are_fired, True) + + def test_infrastrucutre_policy_rule_update(self): + """ + Tests whether a change in the infrastructure policy is detected. + Enriching the metadata policy leads to an update call for this policy. + """ + metadata_policy_changed = copy.deepcopy(DEFAULT_INFRASTRUCTURE_POLICIES[1]) + metadata_policy_changed["rules"][0]["destination_groups"] = ["8.8.8.8"] + metadata_policy_changed["_revision"] = 100 + metadata_policy_changed["rules"][0]["_revision"] = 200 + + with responses.RequestsMock() as rsps: + self._mock_fetch_infrastructure_policy(rsps) + rsps.remove(responses.GET, re.compile(".*Metadata_Allow.*")) + rsps.add( + method=responses.GET, + url=re.compile(".*Metadata_Allow.*"), + body=json.dumps(metadata_policy_changed), + status=200 + ) + rsps.add( + method=responses.PUT, + url=re.compile(".*Metadata_Allow.*"), + status=200, + json=json.dumps(metadata_policy_changed) + ) + self.provider._setup_default_infrastructure_rules() + req_metadata_allow = None + for r in rsps.calls: + if r.request.method == 'PUT': + req_metadata_allow = r + + #Request: GET ICMP_Allow, GET Metadata_Allow, GET DHCP_Allow, PUT Metadata_Allow + self.assertEquals(len(rsps.calls), 4) + self.assertEquals(rsps.assert_all_requests_are_fired, True) + + #Check if the revision number is updated + res = json.loads(req_metadata_allow.request.body) + self.assertEquals(res["_revision"], 100) + self.assertEquals(res["rules"][0]["_revision"], 200) diff --git a/requirements.txt b/requirements.txt index 22c27a7a..9157bc6c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -19,3 +19,4 @@ tooz ratelimiter # Apache-2.0 prometheus_client # Apache-2.0 attrs # MIT License +dictdiffer >=0.7.0