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.

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.")

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)

requirements.txt

@@ -19,3 +19,4 @@ tooz
 ratelimiter                     # Apache-2.0
 prometheus_client               # Apache-2.0
 attrs                           # MIT License
+dictdiffer >=0.7.0