diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py index 6c17ec8e9f4..b35fda68db5 100644 --- a/neutron/db/securitygroups_db.py +++ b/neutron/db/securitygroups_db.py @@ -517,6 +517,19 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, raise ext_sg.SecurityGroupInvalidProtocolForPort( protocol=ip_proto, valid_port_protocols=port_protocols) + def _make_canonical_port_range(self, rule): + if (rule['port_range_min'] == constants.PORT_RANGE_MIN and + rule['port_range_max'] == constants.PORT_RANGE_MAX): + LOG.info('Project %(project)s added a security group rule ' + 'specifying the entire port range (%(min)s - ' + '%(max)s). It was automatically converted to not ' + 'have a range to better optimize it for the backend ' + 'security group implementation(s).', + {'project': rule['tenant_id'], + 'min': rule['port_range_min'], + 'max': rule['port_range_max']}) + rule['port_range_min'] = rule['port_range_max'] = None + def _validate_ethertype_and_protocol(self, rule): """Check if given ethertype and protocol are valid or not""" if rule['protocol'] in [constants.PROTO_NAME_IPV6_ENCAP, @@ -576,6 +589,7 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, def _validate_security_group_rule(self, context, security_group_rule): rule = security_group_rule['security_group_rule'] self._make_canonical_ipv6_icmp_protocol(rule) + self._make_canonical_port_range(rule) self._validate_port_range(rule) self._validate_ip_prefix(rule) self._validate_ethertype_and_protocol(rule) diff --git a/neutron/tests/fullstack/test_securitygroup.py b/neutron/tests/fullstack/test_securitygroup.py index cc9fda6fd24..55b2fe2ae28 100644 --- a/neutron/tests/fullstack/test_securitygroup.py +++ b/neutron/tests/fullstack/test_securitygroup.py @@ -113,13 +113,14 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest): 2. connection from allowed security group is permitted 3. traffic not explicitly allowed (eg. ICMP) is blocked, 4. a security group update takes effect, - 5. a remote security group member addition works, and - 6. an established connection stops by deleting a SG rule. - 7. multiple overlapping remote rules work, - 8. test other protocol functionality by using SCTP protocol - 9. test two vms with same mac on the same host in different - networks - 10. test using multiple security groups + 5. a security group update for entire port range works + 6. a remote security group member addition works, and + 7. an established connection stops by deleting a SG rule. + 8. multiple overlapping remote rules work, + 9. test other protocol functionality by using SCTP protocol + 10. test two vms with same mac on the same host in different + networks + 11. test using multiple security groups """ tenant_uuid = uuidutils.generate_uuid() @@ -172,7 +173,7 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest): vms[1].namespace, vms[0].namespace, vms[0].ip, 3344, net_helpers.NetcatTester.TCP) - self.safe_client.create_security_group_rule( + rule1 = self.safe_client.create_security_group_rule( tenant_uuid, sgs[0]['id'], remote_group_id=sgs[0]['id'], direction='ingress', ethertype=constants.IPv4, @@ -183,7 +184,35 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest): vms[1].namespace, vms[0].namespace, vms[0].ip, 3344, net_helpers.NetcatTester.TCP) - # 5. check if a remote security group member addition works + # 5. check if a security group update for entire port range works + self.client.delete_security_group_rule(rule1['id']) + + self.assert_no_connection( + vms[1].namespace, vms[0].namespace, vms[0].ip, 3344, + net_helpers.NetcatTester.TCP) + + self.assert_no_connection( + vms[1].namespace, vms[0].namespace, vms[0].ip, 3366, + net_helpers.NetcatTester.TCP) + + rule1 = self.safe_client.create_security_group_rule( + tenant_uuid, sgs[0]['id'], + remote_group_id=sgs[0]['id'], direction='ingress', + ethertype=constants.IPv4, + protocol=constants.PROTO_NAME_TCP, + port_range_min=1, port_range_max=65535) + + self.assert_connection( + vms[1].namespace, vms[0].namespace, vms[0].ip, 3344, + net_helpers.NetcatTester.TCP) + + self.assert_connection( + vms[1].namespace, vms[0].namespace, vms[0].ip, 3366, + net_helpers.NetcatTester.TCP) + + self.client.delete_security_group_rule(rule1['id']) + + # 6. check if a remote security group member addition works rule2 = self.safe_client.create_security_group_rule( tenant_uuid, sgs[0]['id'], remote_group_id=sgs[1]['id'], direction='ingress', @@ -195,7 +224,7 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest): vms[2].namespace, vms[0].namespace, vms[0].ip, 3355, net_helpers.NetcatTester.TCP) - # 6. check if an established connection stops by deleting + # 7. check if an established connection stops by deleting # the supporting SG rule. index_to_host.append(index_to_host[2]) self.index_to_sg.append(1) @@ -230,7 +259,7 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest): sleep=8) netcat.stop_processes() - # 7. check if multiple overlapping remote rules work + # 8. check if multiple overlapping remote rules work self.safe_client.create_security_group_rule( tenant_uuid, sgs[0]['id'], remote_group_id=sgs[1]['id'], direction='ingress', @@ -253,7 +282,7 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest): vms[3].namespace, vms[0].namespace, vms[0].ip, 8080, net_helpers.NetcatTester.TCP) - # 8. check SCTP is supported by security group + # 9. check SCTP is supported by security group self.assert_no_connection( vms[1].namespace, vms[0].namespace, vms[0].ip, 3366, net_helpers.NetcatTester.SCTP) @@ -269,10 +298,10 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest): vms[1].namespace, vms[0].namespace, vms[0].ip, 3366, net_helpers.NetcatTester.SCTP) - # 9. test two vms with same mac on the same host in different networks + # 10. test two vms with same mac on the same host in different networks self._test_overlapping_mac_addresses() - # 10. Check using multiple security groups + # 11. Check using multiple security groups self._test_using_multiple_security_groups() def _test_using_multiple_security_groups(self): diff --git a/neutron/tests/unit/extensions/test_securitygroup.py b/neutron/tests/unit/extensions/test_securitygroup.py index 9bed26d47bb..17b47bb0547 100644 --- a/neutron/tests/unit/extensions/test_securitygroup.py +++ b/neutron/tests/unit/extensions/test_securitygroup.py @@ -967,6 +967,27 @@ class TestSecurityGroups(SecurityGroupDBTestCase): for k, v, in keys: self.assertEqual(v, rule['security_group_rule'][k]) + def test_create_security_group_rule_port_range_min_max_limits(self): + name = 'webservers' + description = 'my webservers' + with self.security_group(name, description) as sg: + security_group_id = sg['security_group']['id'] + direction = "ingress" + protocol = const.PROTO_NAME_TCP + port_range_min = const.PORT_RANGE_MIN + port_range_max = const.PORT_RANGE_MAX + # The returned rule should have port range min/max as None + keys = [('security_group_id', security_group_id), + ('direction', direction), + ('protocol', protocol), + ('port_range_min', None), + ('port_range_max', None)] + with self.security_group_rule(security_group_id, direction, + protocol, port_range_min, + port_range_max) as rule: + for k, v, in keys: + self.assertEqual(v, rule['security_group_rule'][k]) + def test_create_security_group_rule_icmp_with_type_and_code(self): name = 'webservers' description = 'my webservers' diff --git a/releasenotes/notes/security-group-rule-all-ports-update-2857d80e5742ebc5.yaml b/releasenotes/notes/security-group-rule-all-ports-update-2857d80e5742ebc5.yaml new file mode 100644 index 00000000000..8383ab85195 --- /dev/null +++ b/releasenotes/notes/security-group-rule-all-ports-update-2857d80e5742ebc5.yaml @@ -0,0 +1,10 @@ +--- +upgrade: + - | + A security group rule added for the entire port range, for example, + TCP ports 1-65535, is not optimal for backends that implement the + rule. Rules like this will now automatically be converted to apply + to the procotol itself, in other words, all TCP - the port ranges + will be ignored. + See bug `1848213 `_ + for more details.