From 0e0c7fa07ef378c580a9a79a9f71b7c15273044a Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Wed, 14 Oct 2020 16:18:36 +0200 Subject: [PATCH] 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 | 2 + neutron/db/securitygroups_db.py | 13 ++++ .../security_groups_normalized_cidr.py | 20 ++++++ neutron/objects/securitygroup.py | 46 ++++++++++++- neutron/plugins/ml2/plugin.py | 2 + neutron/tests/fullstack/test_securitygroup.py | 15 ++++ .../tests/unit/db/test_securitygroups_db.py | 2 +- .../test_security_groups_normalized_cidr.py | 69 +++++++++++++++++++ .../unit/extensions/test_securitygroup.py | 6 ++ neutron/tests/unit/objects/test_objects.py | 4 +- .../tests/unit/objects/test_securitygroup.py | 22 +++++- ...ized-cidr-to-sg-rule-066be9154c2ed0b2.yaml | 7 ++ 12 files changed, 202 insertions(+), 6 deletions(-) create mode 100644 neutron/extensions/security_groups_normalized_cidr.py create mode 100644 neutron/tests/unit/extensions/test_security_groups_normalized_cidr.py create mode 100644 releasenotes/notes/add-normalized-cidr-to-sg-rule-066be9154c2ed0b2.yaml diff --git a/neutron/agent/securitygroups_rpc.py b/neutron/agent/securitygroups_rpc.py index 3c73e1557b3..baf401e1ce2 100644 --- a/neutron/agent/securitygroups_rpc.py +++ b/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.') diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py index d3ea086996f..cbda2585255 100644 --- a/neutron/db/securitygroups_db.py +++ b/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. diff --git a/neutron/extensions/security_groups_normalized_cidr.py b/neutron/extensions/security_groups_normalized_cidr.py new file mode 100644 index 00000000000..d3bec49905f --- /dev/null +++ b/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 diff --git a/neutron/objects/securitygroup.py b/neutron/objects/securitygroup.py index 7a7986509dd..59c3c60b4e4 100644 --- a/neutron/objects/securitygroup.py +++ b/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 diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index eb0e579dbc1..23018ed5d05 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/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, diff --git a/neutron/tests/fullstack/test_securitygroup.py b/neutron/tests/fullstack/test_securitygroup.py index 0946f890bf1..3ed8d523e7c 100644 --- a/neutron/tests/fullstack/test_securitygroup.py +++ b/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']) diff --git a/neutron/tests/unit/db/test_securitygroups_db.py b/neutron/tests/unit/db/test_securitygroups_db.py index a064a8beaaf..926fe253dc7 100644 --- a/neutron/tests/unit/db/test_securitygroups_db.py +++ b/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', diff --git a/neutron/tests/unit/extensions/test_security_groups_normalized_cidr.py b/neutron/tests/unit/extensions/test_security_groups_normalized_cidr.py new file mode 100644 index 00000000000..5dee1820b63 --- /dev/null +++ b/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'] + ) diff --git a/neutron/tests/unit/extensions/test_securitygroup.py b/neutron/tests/unit/extensions/test_securitygroup.py index 2d0bb60a8ab..2ada04da99a 100644 --- a/neutron/tests/unit/extensions/test_securitygroup.py +++ b/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): diff --git a/neutron/tests/unit/objects/test_objects.py b/neutron/tests/unit/objects/test_objects.py index a6022d39fb6..71503e79019 100644 --- a/neutron/tests/unit/objects/test_objects.py +++ b/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', diff --git a/neutron/tests/unit/objects/test_securitygroup.py b/neutron/tests/unit/objects/test_securitygroup.py index bbfe14e3fef..192cf86c75a 100644 --- a/neutron/tests/unit/objects/test_securitygroup.py +++ b/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']) diff --git a/releasenotes/notes/add-normalized-cidr-to-sg-rule-066be9154c2ed0b2.yaml b/releasenotes/notes/add-normalized-cidr-to-sg-rule-066be9154c2ed0b2.yaml new file mode 100644 index 00000000000..74701070b84 --- /dev/null +++ b/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.