diff --git a/gbpservice/neutron/db/migration/alembic_migrations/versions/7eaf537d927f_sg_rule_remote_group_id_insertion.py b/gbpservice/neutron/db/migration/alembic_migrations/versions/7eaf537d927f_sg_rule_remote_group_id_insertion.py new file mode 100644 index 000000000..25e29e67d --- /dev/null +++ b/gbpservice/neutron/db/migration/alembic_migrations/versions/7eaf537d927f_sg_rule_remote_group_id_insertion.py @@ -0,0 +1,54 @@ +# 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. +# + +"""sg_rule remote_group_id insertion + +Revision ID: 7eaf537d927f +Revises: f28141ea1bbf +Create Date: 2020-05-06 00:00:00.000000 + +""" + +# revision identifiers, used by Alembic. +revision = '7eaf537d927f' +down_revision = 'f28141ea1bbf' + +from alembic import op +from alembic import util +import sqlalchemy as sa + + +def upgrade(): + # See if AIM is being used, and if so, migrate data. + bind = op.get_bind() + insp = sa.engine.reflection.Inspector.from_engine(bind) + if 'aim_tenants' in insp.get_table_names(): + try: + # Note - this cannot be imported unless we know the + # apic_aim mechanism driver is deployed, since the AIM + # library may not be installed. + from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import ( + data_migrations) + + session = sa.orm.Session(bind=bind, autocommit=True) + data_migrations.do_sg_rule_remote_group_id_insertion(session) + except ImportError: + util.warn("AIM schema present, but failed to import AIM libraries" + " - SG rules remote_group_id not inserted.") + except Exception as e: + util.warn("Caught exception inserting SG rules remote_group_id: %s" + % e) + + +def downgrade(): + pass diff --git a/gbpservice/neutron/db/migration/alembic_migrations/versions/HEAD b/gbpservice/neutron/db/migration/alembic_migrations/versions/HEAD index 1b4974d70..d2ebed32b 100644 --- a/gbpservice/neutron/db/migration/alembic_migrations/versions/HEAD +++ b/gbpservice/neutron/db/migration/alembic_migrations/versions/HEAD @@ -1 +1 @@ -f28141ea1bbf +7eaf537d927f diff --git a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/data_migrations.py b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/data_migrations.py index 37f917744..5f7977c57 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/data_migrations.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/data_migrations.py @@ -386,3 +386,32 @@ def do_apic_aim_security_group_migration(session): alembic_util.msg( "Finished data migration for SGs and its rules.") + + +def do_sg_rule_remote_group_id_insertion(session): + alembic_util.msg( + "Starting remote_group_id insertion for SG rules.") + + aim = aim_manager.AimManager() + aim_ctx = aim_context.AimContext(session) + mapper = apic_mapper.APICNameMapper() + with session.begin(subtransactions=True): + sg_rule_dbs = (session.query(sg_models.SecurityGroupRule). + options(lazyload('*')).all()) + for sg_rule_db in sg_rule_dbs: + if sg_rule_db.get('remote_group_id'): + tenant_aname = mapper.project(session, sg_rule_db['tenant_id']) + sg_rule_aim = aim_resource.SecurityGroupRule( + tenant_name=tenant_aname, + security_group_name=sg_rule_db['security_group_id'], + security_group_subject_name='default', + name=sg_rule_db['id']) + sg_rule_aim = aim.get(aim_ctx, sg_rule_aim) + # Validation tool will add the missing SG rules + # if there is any. + if sg_rule_aim: + aim.update(aim_ctx, sg_rule_aim, + remote_group_id=sg_rule_db['remote_group_id']) + + alembic_util.msg( + "Finished remote_group_id insertion for SG rules.") diff --git a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py index f557b926d..eb4da521f 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py @@ -2485,7 +2485,9 @@ class ApicMechanismDriver(api_plus.MechanismDriver, from_port=(sg_rule['port_range_min'] if sg_rule['port_range_min'] else 'unspecified'), to_port=(sg_rule['port_range_max'] - if sg_rule['port_range_max'] else 'unspecified')) + if sg_rule['port_range_max'] else 'unspecified'), + remote_group_id=(sg_rule['remote_group_id'] + if sg_rule['remote_group_id'] else '')) self.aim.create(aim_ctx, sg_rule_aim) def update_security_group_precommit(self, context): @@ -2549,9 +2551,12 @@ class ApicMechanismDriver(api_plus.MechanismDriver, for sg_port in sg_ports: for fixed_ip in sg_port['fixed_ips']: remote_ips.append(fixed_ip['ip_address']) + + remote_group_id = sg_rule['remote_group_id'] else: remote_ips = ([sg_rule['remote_ip_prefix']] if sg_rule['remote_ip_prefix'] else '') + remote_group_id = '' sg_rule_aim = aim_resource.SecurityGroupRule( tenant_name=tenant_aname, @@ -2573,7 +2578,8 @@ class ApicMechanismDriver(api_plus.MechanismDriver, from_port=(sg_rule['port_range_min'] if sg_rule['port_range_min'] else 'unspecified'), to_port=(sg_rule['port_range_max'] - if sg_rule['port_range_max'] else 'unspecified')) + if sg_rule['port_range_max'] else 'unspecified'), + remote_group_id=remote_group_id) self.aim.create(aim_ctx, sg_rule_aim) def delete_security_group_rule_precommit(self, context): @@ -5820,7 +5826,9 @@ class ApicMechanismDriver(api_plus.MechanismDriver, mgr.expect_aim_resource(sg_subject) for rule_db in sg_db.rules: remote_ips = [] + remote_group_id = '' if rule_db.remote_group_id: + remote_group_id = rule_db.remote_group_id ip_version = (4 if rule_db.ethertype == 'IPv4' else 6 if rule_db.ethertype == 'IPv6' else 0) @@ -5843,7 +5851,8 @@ class ApicMechanismDriver(api_plus.MechanismDriver, else 'unspecified'), to_port=(rule_db.port_range_max if rule_db.port_range_max - else 'unspecified')) + else 'unspecified'), + remote_group_id=remote_group_id) mgr.expect_aim_resource(sg_rule) def _validate_ports(self, mgr): diff --git a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py index 542d78b35..f321065a5 100644 --- a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py +++ b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py @@ -559,6 +559,69 @@ class ApicAimTestCase(test_address_scope.AddressScopeTestCase, return mock.DEFAULT return verify + def _get_tenant(self, tenant_name): + session = db_api.get_session() + aim_ctx = aim_context.AimContext(session) + tenant = aim_resource.Tenant(name=tenant_name) + tenant = self.aim_mgr.get(aim_ctx, tenant) + self.assertIsNotNone(tenant) + return tenant + + def _check_sg(self, sg): + tenant_aname = self.name_mapper.project(None, sg['tenant_id']) + self._get_tenant(tenant_aname) + + aim_sg = self._get_sg(sg['id'], tenant_aname) + self.assertEqual(tenant_aname, aim_sg.tenant_name) + self.assertEqual(sg['id'], aim_sg.name) + self.assertEqual(sg['name'], aim_sg.display_name) + + # check those SG rules including the default ones + for sg_rule in sg.get('security_group_rules', []): + self._check_sg_rule(sg['id'], sg_rule) + + def _check_sg_rule(self, sg_id, sg_rule): + tenant_aname = self.name_mapper.project(None, sg_rule['tenant_id']) + self._get_tenant(tenant_aname) + + aim_sg_rule = self._get_sg_rule( + sg_rule['id'], 'default', sg_id, tenant_aname) + + self.assertEqual(tenant_aname, aim_sg_rule.tenant_name) + self.assertEqual(sg_id, aim_sg_rule.security_group_name) + self.assertEqual('default', + aim_sg_rule.security_group_subject_name) + self.assertEqual(sg_rule['id'], aim_sg_rule.name) + self.assertEqual(sg_rule['direction'], aim_sg_rule.direction) + self.assertEqual(sg_rule['ethertype'].lower(), + aim_sg_rule.ethertype) + self.assertEqual('reflexive', aim_sg_rule.conn_track) + self.assertEqual(([sg_rule['remote_ip_prefix']] if + sg_rule['remote_ip_prefix'] else []), + aim_sg_rule.remote_ips) + self.assertEqual((sg_rule['remote_group_id'] if + sg_rule['remote_group_id'] else ''), + aim_sg_rule.remote_group_id) + self.assertEqual(str(self.driver.get_aim_protocol( + sg_rule['protocol'])), str(aim_sg_rule.ip_protocol)) + self.assertEqual((str(sg_rule['port_range_min']) if + sg_rule['port_range_min'] else 'unspecified'), + aim_sg_rule.from_port) + self.assertEqual((str(sg_rule['port_range_max']) if + sg_rule['port_range_max'] else 'unspecified'), + aim_sg_rule.to_port) + if (sg_rule['protocol'] and sg_rule['protocol'].lower() == 'icmp'): + if (sg_rule['port_range_min']): + self.assertEqual(str(sg_rule['port_range_min']), + aim_sg_rule.icmp_code) + else: + self.assertEqual(aim_sg_rule.icmp_code, 'unspecified') + if (sg_rule['port_range_max']): + self.assertEqual(str(sg_rule['port_range_max']), + aim_sg_rule.icmp_type) + else: + self.assertEqual(aim_sg_rule.icmp_type, 'unspecified') + class TestRpcListeners(ApicAimTestCase): @staticmethod @@ -734,14 +797,6 @@ class TestAimMapping(ApicAimTestCase): self.call_wrapper.tearDown() super(TestAimMapping, self).tearDown() - def _get_tenant(self, tenant_name): - session = db_api.get_session() - aim_ctx = aim_context.AimContext(session) - tenant = aim_resource.Tenant(name=tenant_name) - tenant = self.aim_mgr.get(aim_ctx, tenant) - self.assertIsNotNone(tenant) - return tenant - def _get_vrf(self, vrf_name, tenant_name, should_exist=True): session = db_api.get_session() aim_ctx = aim_context.AimContext(session) @@ -1050,58 +1105,6 @@ class TestAimMapping(ApicAimTestCase): aname = self.name_mapper.address_scope(None, scope['id']) self._vrf_should_not_exist(aname) - def _check_sg(self, sg): - tenant_aname = self.name_mapper.project(None, sg['tenant_id']) - self._get_tenant(tenant_aname) - - aim_sg = self._get_sg(sg['id'], tenant_aname) - self.assertEqual(tenant_aname, aim_sg.tenant_name) - self.assertEqual(sg['id'], aim_sg.name) - self.assertEqual(sg['name'], aim_sg.display_name) - - # check those SG rules including the default ones - for sg_rule in sg.get('security_group_rules', []): - self._check_sg_rule(sg['id'], sg_rule) - - def _check_sg_rule(self, sg_id, sg_rule): - tenant_aname = self.name_mapper.project(None, sg_rule['tenant_id']) - self._get_tenant(tenant_aname) - - aim_sg_rule = self._get_sg_rule( - sg_rule['id'], 'default', sg_id, tenant_aname) - - self.assertEqual(tenant_aname, aim_sg_rule.tenant_name) - self.assertEqual(sg_id, aim_sg_rule.security_group_name) - self.assertEqual('default', - aim_sg_rule.security_group_subject_name) - self.assertEqual(sg_rule['id'], aim_sg_rule.name) - self.assertEqual(sg_rule['direction'], aim_sg_rule.direction) - self.assertEqual(sg_rule['ethertype'].lower(), - aim_sg_rule.ethertype) - self.assertEqual('reflexive', aim_sg_rule.conn_track) - self.assertEqual(([sg_rule['remote_ip_prefix']] if - sg_rule['remote_ip_prefix'] else []), - aim_sg_rule.remote_ips) - self.assertEqual(str(self.driver.get_aim_protocol( - sg_rule['protocol'])), str(aim_sg_rule.ip_protocol)) - self.assertEqual((str(sg_rule['port_range_min']) if - sg_rule['port_range_min'] else 'unspecified'), - aim_sg_rule.from_port) - self.assertEqual((str(sg_rule['port_range_max']) if - sg_rule['port_range_max'] else 'unspecified'), - aim_sg_rule.to_port) - if (sg_rule['protocol'] and sg_rule['protocol'].lower() == 'icmp'): - if (sg_rule['port_range_min']): - self.assertEqual(str(sg_rule['port_range_min']), - aim_sg_rule.icmp_code) - else: - self.assertEqual(aim_sg_rule.icmp_code, 'unspecified') - if (sg_rule['port_range_max']): - self.assertEqual(str(sg_rule['port_range_max']), - aim_sg_rule.icmp_type) - else: - self.assertEqual(aim_sg_rule.icmp_type, 'unspecified') - def _check_router_deleted(self, router): aname = self.name_mapper.router(None, router['id']) self._subject_should_not_exist('route', aname) @@ -4719,6 +4722,43 @@ class TestMigrations(ApicAimTestCase, db.DbMixin): sgrules = self.aim_mgr.find(aim_ctx, aim_resource.SecurityGroupRule) self.assertIsNotNone(sgrules) + def test_sg_rule_remote_group_id_insertion(self): + sg = self._make_security_group(self.fmt, + 'sg1', 'test')['security_group'] + # Add 2 more rules. + rule1 = self._build_security_group_rule( + sg['id'], 'ingress', n_constants.PROTO_NAME_TCP, '22', '23', + remote_ip_prefix="1.1.1.0/24", remote_group_id=None, + ethertype=n_constants.IPv4) + rules = {'security_group_rules': [rule1['security_group_rule']]} + sg_rule1 = self._make_security_group_rule( + self.fmt, rules)['security_group_rules'][0] + + rule2 = self._build_security_group_rule( + sg['id'], 'egress', n_constants.PROTO_NAME_TCP, '44', '45', + remote_ip_prefix=None, remote_group_id=sg['id'], + ethertype=n_constants.IPv6) + rules = {'security_group_rules': [rule2['security_group_rule']]} + sg_rule2 = self._make_security_group_rule( + self.fmt, rules)['security_group_rules'][0] + + # It will check all the sg_rules inside _check_sg(). + self._check_sg(sg) + for sg_rule in [sg_rule1, sg_rule2]: + self._check_sg_rule(sg['id'], sg_rule) + + # Wipe out the remote_group_id of all the sg_rules. + aim_ctx = aim_context.AimContext(self.db_session) + sg_rules = self.aim_mgr.find(aim_ctx, aim_resource.SecurityGroupRule) + for sg_rule in sg_rules: + self.aim_mgr.update(aim_ctx, sg_rule, remote_group_id='') + + data_migrations.do_sg_rule_remote_group_id_insertion(self.db_session) + + self._check_sg(sg) + for sg_rule in [sg_rule1, sg_rule2]: + self._check_sg_rule(sg['id'], sg_rule) + class TestPortBinding(ApicAimTestCase): def test_bind_opflex_agent(self): @@ -8514,6 +8554,8 @@ class TestPortOnPhysicalNode(TestPortVlanNetwork): aim_sg_rule = self._get_sg_rule( sg_rule['id'], 'default', default_sg_id, tenant_aname) self.assertEqual(aim_sg_rule.remote_ips, ['10.0.1.100']) + self.assertEqual( + aim_sg_rule.remote_group_id, sg_rule['remote_group_id']) # add another rule with remote_group_id set rule1 = self._build_security_group_rule( @@ -8525,6 +8567,8 @@ class TestPortOnPhysicalNode(TestPortVlanNetwork): aim_sg_rule = self._get_sg_rule( sg_rule1['id'], 'default', default_sg_id, tenant_aname) self.assertEqual(aim_sg_rule.remote_ips, ['10.0.1.100']) + self.assertEqual( + aim_sg_rule.remote_group_id, sg_rule1['remote_group_id']) rule2 = self._build_security_group_rule( default_sg_id, 'ingress', n_constants.PROTO_NAME_ICMP, '2', '33',