Add normalized_cidr column to SG rules

New API extension was added in [1] to extend security group rules with
"normalized_cidr" read only attribute.
This patch implements this API extension in Neutron ML2 plugin and
extends security group rules with "normalized_cidr" value.

[1] https://review.opendev.org/#/c/743630/

Related-Bug: #1869129

Change-Id: I65584817a22f952da8da979ab68cd6cfaa2143be

neutron/agent/securitygroups_rpc.py

@@ -17,6 +17,7 @@
 import functools
 
 from neutron_lib.api.definitions import rbac_security_groups as rbac_sg_apidef
+from neutron_lib.api.definitions import security_groups_normalized_cidr
 from neutron_lib.api.definitions import security_groups_remote_address_group \
     as sgag_def
 from neutron_lib.api.definitions import stateful_security_group as stateful_sg
@@ -52,6 +53,7 @@ def disable_security_group_extension_by_config(aliases):
         _disable_extension(rbac_sg_apidef.ALIAS, aliases)
         _disable_extension(stateful_sg.ALIAS, aliases)
         _disable_extension(sgag_def.ALIAS, aliases)
+        _disable_extension(security_groups_normalized_cidr.ALIAS, aliases)
         LOG.info('Disabled allowed-address-pairs extension.')
         _disable_extension('allowed-address-pairs', aliases)
         LOG.info('Disabled address-group extension.')

neutron/db/securitygroups_db.py

@@ -667,6 +667,8 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase,
         return sg_id
 
     def _make_security_group_rule_dict(self, security_group_rule, fields=None):
+
+        # TODO(slaweq): switch this to use OVO instead of db object
         res = {'id': security_group_rule['id'],
                'tenant_id': security_group_rule['tenant_id'],
                'security_group_id': security_group_rule['security_group_id'],
@@ -678,6 +680,8 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase,
                'remote_ip_prefix': security_group_rule['remote_ip_prefix'],
                'remote_address_group_id': security_group_rule[
                    'remote_address_group_id'],
+               'normalized_cidr': self._get_normalized_cidr_from_rule(
+                   security_group_rule),
                'remote_group_id': security_group_rule['remote_group_id'],
                'standard_attr_id': security_group_rule.standard_attr.id,
                }
@@ -686,6 +690,15 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase,
                                     security_group_rule)
         return db_utils.resource_fields(res, fields)
 
+    @staticmethod
+    def _get_normalized_cidr_from_rule(rule):
+        normalized_cidr = None
+        remote_ip_prefix = rule.get('remote_ip_prefix')
+        if remote_ip_prefix:
+            normalized_cidr = str(
+                net.AuthenticIPNetwork(remote_ip_prefix).cidr)
+        return normalized_cidr
+
     def _rule_to_key(self, rule):
         def _normalize_rule_value(key, value):
             # This string is used as a placeholder for str(None), but shorter.

neutron/extensions/security_groups_normalized_cidr.py

@@ -0,0 +1,20 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from neutron_lib.api.definitions import security_groups_normalized_cidr
+from neutron_lib.api import extensions
+
+
+class Security_groups_normalized_cidr(extensions.APIExtensionDescriptor):
+    """Extension class supporting normalized cidr in the SG rules."""
+
+    api_definition = security_groups_normalized_cidr

neutron/objects/securitygroup.py

@@ -39,7 +39,8 @@ class SecurityGroup(rbac_db.NeutronRbacObject):
     # Version 1.1: Add RBAC support
     # Version 1.2: Added stateful support
     # Version 1.3: Added support for remote_address_group_id in rules
-    VERSION = '1.3'
+    # Version 1.4: Added support for normalized_cidr in rules
+    VERSION = '1.4'
 
     # required by RbacNeutronMetaclass
     rbac_db_cls = SecurityGroupRBAC
@@ -102,6 +103,16 @@ class SecurityGroup(rbac_db.NeutronRbacObject):
                     rule['versioned_object.data'], '1.0')
                 rule['versioned_object.version'] = '1.0'
 
+        def filter_normalized_cidr_from_rules(rules):
+            sg_rule = SecurityGroupRule()
+            for rule in rules:
+                rule_version = versionutils.convert_version_to_tuple(
+                    rule['versioned_object.version'])
+                if rule_version > (1, 1):
+                    sg_rule.obj_make_compatible(
+                        rule['versioned_object.data'], '1.1')
+                    rule['versioned_object.version'] = '1.1'
+
         if _target_version < (1, 1):
             primitive.pop('shared')
         if _target_version < (1, 2):
@@ -109,6 +120,9 @@ class SecurityGroup(rbac_db.NeutronRbacObject):
         if _target_version < (1, 3):
             if 'rules' in primitive:
                 filter_remote_address_group_id_from_rules(primitive['rules'])
+        if _target_version < (1, 4):
+            if 'rules' in primitive:
+                filter_normalized_cidr_from_rules(primitive['rules'])
 
     @classmethod
     def get_bound_tenant_ids(cls, context, obj_id):
@@ -138,7 +152,8 @@ class DefaultSecurityGroup(base.NeutronDbObject):
 class SecurityGroupRule(base.NeutronDbObject):
     # Version 1.0: Initial version
     # Version 1.1: Add remote address group support
-    VERSION = '1.1'
+    # Version 1.2: Added normalized cidr column
+    VERSION = '1.2'
 
     db_model = sg_models.SecurityGroupRule
 
@@ -154,8 +169,11 @@ class SecurityGroupRule(base.NeutronDbObject):
         'port_range_max': common_types.PortRangeWith0Field(nullable=True),
         'remote_ip_prefix': common_types.IPNetworkField(nullable=True),
         'remote_address_group_id': common_types.UUIDField(nullable=True),
+        'normalized_cidr': common_types.IPNetworkField(nullable=True),
     }
 
+    synthetic_fields = ['normalized_cidr']
+
     foreign_keys = {'SecurityGroup': {'security_group_id': 'id'}}
 
     fields_no_update = ['project_id', 'security_group_id', 'remote_group_id',
@@ -165,6 +183,8 @@ class SecurityGroupRule(base.NeutronDbObject):
         _target_version = versionutils.convert_version_to_tuple(target_version)
         if _target_version < (1, 1):
             primitive.pop('remote_address_group_id', None)
+        if _target_version < (1, 2):
+            primitive.pop('normalized_cidr', None)
 
     # TODO(sayalilunkad): get rid of it once we switch the db model to using
     # custom types.
@@ -176,6 +196,28 @@ class SecurityGroupRule(base.NeutronDbObject):
             result['remote_ip_prefix'] = cls.filter_to_str(remote_ip_prefix)
         return result
 
+    def _load_normalized_cidr(self, db_obj=None):
+        db_obj = db_obj or SecurityGroupRule.get_object(self.obj_context,
+                                                        id=self.id)
+        if not db_obj:
+            return
+
+        cidr = None
+        if db_obj.remote_ip_prefix:
+            cidr = net_utils.AuthenticIPNetwork(db_obj.remote_ip_prefix).cidr
+
+        setattr(self, 'normalized_cidr', cidr)
+        self.obj_reset_changes(['normalized_cidr'])
+
+    def from_db_object(self, db_obj):
+        super(SecurityGroupRule, self).from_db_object(db_obj)
+        self._load_normalized_cidr(db_obj)
+
+    def obj_load_attr(self, attrname):
+        if attrname == 'normalized_cidr':
+            return self._load_normalized_cidr()
+        super(SecurityGroupRule, self).obj_load_attr(attrname)
+
     # TODO(sayalilunkad): get rid of it once we switch the db model to using
     # custom types.
     @classmethod

neutron/plugins/ml2/plugin.py

@@ -48,6 +48,7 @@ from neutron_lib.api.definitions import provider_net
 from neutron_lib.api.definitions import rbac_address_scope
 from neutron_lib.api.definitions import rbac_security_groups as rbac_sg_apidef
 from neutron_lib.api.definitions import rbac_subnetpool
+from neutron_lib.api.definitions import security_groups_normalized_cidr
 from neutron_lib.api.definitions import security_groups_port_filtering
 from neutron_lib.api.definitions import security_groups_remote_address_group
 from neutron_lib.api.definitions import stateful_security_group
@@ -206,6 +207,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                                     default_subnetpools.ALIAS,
                                     "subnet-service-types",
                                     ip_substring_port_filtering.ALIAS,
+                                    security_groups_normalized_cidr.ALIAS,
                                     security_groups_port_filtering.ALIAS,
                                     security_groups_remote_address_group.ALIAS,
                                     empty_string_filtering.ALIAS,

neutron/tests/fullstack/test_securitygroup.py

@@ -691,3 +691,18 @@ class SecurityGroupRulesTest(base.BaseFullStackTestCase):
 
         self.assertRaises(nc_exc.OverQuotaClient,
                           self.safe_client.create_security_group, project_id)
+
+    def test_normalized_cidr_in_rule(self):
+        project_id = uuidutils.generate_uuid()
+        sg = self.safe_client.create_security_group(project_id)
+
+        rule = self.safe_client.create_security_group_rule(
+            project_id, sg['id'], direction='ingress',
+            remote_ip_prefix='10.0.0.34/24')
+        self.assertEqual('10.0.0.0/24', rule['normalized_cidr'])
+        self.assertEqual('10.0.0.34/24', rule['remote_ip_prefix'])
+
+        rule = self.safe_client.create_security_group_rule(
+            project_id, sg['id'], direction='ingress')
+        self.assertIsNone(rule['normalized_cidr'])
+        self.assertIsNone(rule['remote_ip_prefix'])

neutron/tests/unit/db/test_securitygroups_db.py

@@ -125,7 +125,7 @@ class SecurityGroupDbMixinTestCase(testlib_api.SqlTestCase):
             with testtools.ExpectedException(
                     securitygroup.SecurityGroupConflict):
                 self.mixin.create_security_group_rule(
-                    self.ctx, mock.MagicMock())
+                    self.ctx, FAKE_SECGROUP_RULE)
 
     def test__check_for_duplicate_rules_does_not_drop_protocol(self):
         with mock.patch.object(self.mixin, 'get_security_group',

neutron/tests/unit/extensions/test_security_groups_normalized_cidr.py

@@ -0,0 +1,69 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from neutron_lib.api.definitions import security_groups_normalized_cidr
+import webob.exc
+
+from neutron.tests.unit.extensions import test_securitygroup
+
+
+DB_PLUGIN_KLASS = (
+    'neutron.tests.unit.extensions.test_security_groups_normalized_cidr.'
+    'TestPlugin')
+
+
+class SecurityGroupNormalizedCidrTestExtManager(
+        test_securitygroup.SecurityGroupTestExtensionManager):
+
+    def get_resources(self):
+        self.update_attributes_map(
+            security_groups_normalized_cidr.RESOURCE_ATTRIBUTE_MAP)
+        return super(
+            SecurityGroupNormalizedCidrTestExtManager, self).get_resources()
+
+
+class TestPlugin(test_securitygroup.SecurityGroupTestPlugin):
+
+    supported_extension_aliases = ['security-group',
+                                   security_groups_normalized_cidr.ALIAS]
+
+
+class TestSecurityGroupsNormalizedCidr(
+        test_securitygroup.SecurityGroupDBTestCase):
+
+    def setUp(self):
+        super(TestSecurityGroupsNormalizedCidr, self).setUp(
+              plugin=DB_PLUGIN_KLASS,
+              ext_mgr=SecurityGroupNormalizedCidrTestExtManager())
+
+    def test_create_security_group_rule_with_not_normalized_cidr(self):
+        name = 'webservers'
+        description = 'my webservers'
+        remote_prefixes = ['10.0.0.120/24', '10.0.0.200/24']
+        with self.security_group(name, description) as sg:
+            sg_id = sg['security_group']['id']
+            for remote_ip_prefix in remote_prefixes:
+                rule = self._build_security_group_rule(
+                    sg_id,
+                    'ingress', 'tcp',
+                    remote_ip_prefix=remote_ip_prefix)
+                res = self._create_security_group_rule(self.fmt, rule)
+                self.assertEqual(webob.exc.HTTPCreated.code, res.status_int)
+                res_sg = self.deserialize(self.fmt, res)
+                self.assertEqual(
+                    '10.0.0.0/24',
+                    res_sg['security_group_rule']['normalized_cidr']
+                )
+                self.assertEqual(
+                    remote_ip_prefix,
+                    res_sg['security_group_rule']['remote_ip_prefix']
+                )

neutron/tests/unit/extensions/test_securitygroup.py

@@ -77,6 +77,12 @@ class SecurityGroupTestExtensionManager(object):
     def get_request_extensions(self):
         return []
 
+    def update_attributes_map(self, attributes):
+        for resource, attrs in ext_sg.RESOURCE_ATTRIBUTE_MAP.items():
+            extended_attrs = attributes.get(resource)
+            if extended_attrs:
+                attrs.update(extended_attrs)
+
 
 class SecurityGroupsTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
 

neutron/tests/unit/objects/test_objects.py

@@ -102,10 +102,10 @@ object_data = {
     'RouterL3AgentBinding': '1.0-c5ba6c95e3a4c1236a55f490cd67da82',
     'RouterPort': '1.0-c8c8f499bcdd59186fcd83f323106908',
     'RouterRoute': '1.0-07fc5337c801fb8c6ccfbcc5afb45907',
-    'SecurityGroup': '1.3-7b63b834e511856f54a09282d6843ecc',
+    'SecurityGroup': '1.4-7b63b834e511856f54a09282d6843ecc',
     'SecurityGroupPortBinding': '1.0-6879d5c0af80396ef5a72934b6a6ef20',
     'SecurityGroupRBAC': '1.0-192845c5ed0718e1c54fac36936fcd7d',
-    'SecurityGroupRule': '1.1-0a8614633901e353dd32948dc2f8708f',
+    'SecurityGroupRule': '1.2-27793368d4ac35f2ed6e0bb653c6aaad',
     'SegmentHostMapping': '1.0-521597cf82ead26217c3bd10738f00f0',
     'ServiceProfile': '1.0-9beafc9e7d081b8258f3c5cb66ac5eed',
     'StandardAttribute': '1.0-617d4f46524c4ce734a6fc1cc0ac6a0b',

neutron/tests/unit/objects/test_securitygroup.py

@@ -13,6 +13,7 @@
 import collections
 import itertools
 
+import netaddr
 from oslo_utils import uuidutils
 
 from neutron.objects import securitygroup
@@ -72,6 +73,11 @@ class SecurityGroupDbObjTestCase(test_base.BaseDbObjectTestCase,
                 # generated; we picked the former here
                 rule['remote_group_id'] = None
 
+        sg_rule = self.get_random_db_fields(securitygroup.SecurityGroupRule)
+        self.model_map.update({
+            self._test_class.db_model: self.db_objs,
+            securitygroup.SecurityGroupRule.db_model: sg_rule})
+
     def _create_test_security_group(self):
         self.objs[0].create()
         return self.objs[0]
@@ -81,7 +87,8 @@ class SecurityGroupDbObjTestCase(test_base.BaseDbObjectTestCase,
         rule_params = {
             'project_id': sg_obj.project_id,
             'security_group_id': sg_obj.id,
-            'remote_address_group_id': None}
+            'remote_address_group_id': None,
+            'remote_ip_prefix': netaddr.IPNetwork('10.0.0.120/24')}
         sg_rule = securitygroup.SecurityGroupRule(
             self.context, **rule_params)
         sg_obj.rules = [sg_rule]
@@ -95,6 +102,13 @@ class SecurityGroupDbObjTestCase(test_base.BaseDbObjectTestCase,
             self.assertNotIn('remote_address_group_id',
                              rule['versioned_object.data'])
 
+    def test_object_version_degradation_1_4_to_1_3_no_normalized_cidr(self):
+        sg_obj = self._create_test_security_group_with_rule()
+        sg_obj_1_3 = sg_obj.obj_to_primitive('1.3')
+        for rule in sg_obj_1_3['versioned_object.data']['rules']:
+            self.assertEqual('1.1', rule['versioned_object.version'])
+            self.assertNotIn('normalized_cidr', rule['versioned_object.data'])
+
     def test_object_version_degradation_1_2_to_1_1_no_stateful(self):
         sg_stateful_obj = self._create_test_security_group()
         sg_no_stateful_obj = sg_stateful_obj.obj_to_primitive('1.1')
@@ -281,3 +295,9 @@ class SecurityGroupRuleDbObjTestCase(test_base.BaseDbObjectTestCase,
         rule_no_remote_ag_obj = rule_remote_ag_obj.obj_to_primitive('1.0')
         self.assertNotIn('remote_address_group_id',
                          rule_no_remote_ag_obj['versioned_object.data'])
+
+    def test_object_version_degradation_1_2_to_1_1_no_normalized_cidr(self):
+        sg_rule_obj = self._create_test_security_group_rule()
+        sg_rule_10_obj = sg_rule_obj.obj_to_primitive('1.0')
+        self.assertNotIn('normalized_cidr',
+                         sg_rule_10_obj['versioned_object.data'])

releasenotes/notes/add-normalized-cidr-to-sg-rule-066be9154c2ed0b2.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Security group rule has now new, read only attribute ``normalized_cidr``
+    which contains network address from the CIDR provided in the
+    ``remote_ip_prefix`` attribute.
+    This new attribute shows actual CIDR used by backend firewall drivers.