openstack/shade
Code Issues Proposed changes

Modify secgroup rule processing

Browse Source 
It turns out that Neutron can accept None for a rule protocol to
represent all protocols. Nova has no similar concept for a single
rule, so let's throw an exception.

Also, Neutron relaxes the rules on accepting None for both port
values to represent the full port range for TCP and UDP. For ICMP,
it is the same as -1.

Change-Id: Icc408586804e2a538399a2aa3f740b97bb4c5246
This commit is contained in:
David Shrewsbury
2015-06-30 16:35:55 -04:00
parent d729770ea4
commit 01aeceeae9
2 changed files with 46 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
shade/__init__.py
View File

@@ -3100,12 +3100,28 @@ class OpenStackCloud(object):
return rule['security_group_rule']
elif self.secgroup_source == 'nova':
# NOTE: Neutron accepts None for ports, but Nova accepts -1
# as the equivalent value.
if port_range_min is None:
port_range_min = -1
if port_range_max is None:
port_range_max = -1
# NOTE: Neutron accepts None for protocol. Nova does not.
if protocol is None:
raise OpenStackCloudException('Protocol must be specified')
# NOTE: Neutron accepts None for ports, but Nova requires -1
# as the equivalent value for ICMP.
#
# For TCP/UDP, if both are None, Neutron allows this and Nova
# represents this as all ports (1-65535). Nova does not accept
# None values, so to hide this difference, we will automatically
# convert to the full port range. If only a single port value is
# specified, it will error as normal.
if protocol == 'icmp':
if port_range_min is None:
port_range_min = -1
if port_range_max is None:
port_range_max = -1
elif protocol in ['tcp', 'udp']:
if port_range_min is None and port_range_max is None:
port_range_min = 1
port_range_max = 65535
try:
rule = meta.obj_to_dict(
self.manager.submitTask(

27
shade/tests/unit/test_security_groups.py
View File

@@ -235,20 +235,41 @@ class TestSecurityGroups(base.TestCase):
self.cloud.secgroup_source = 'nova'
new_rule = fakes.FakeNovaSecgroupRule(
id='xyz', from_port=-1, to_port=2000, ip_protocol='tcp',
id='xyz', from_port=1, to_port=2000, ip_protocol='tcp',
cidr='1.2.3.4/32')
mock_nova.security_group_rules.create.return_value = new_rule
mock_get.return_value = {'id': 'abc'}
self.cloud.create_security_group_rule(
'abc', port_range_max=2000, protocol='tcp',
'abc', port_range_min=1, port_range_max=2000, protocol='tcp',
remote_ip_prefix='1.2.3.4/32', remote_group_id='123')
mock_nova.security_group_rules.create.assert_called_once_with(
parent_group_id='abc', ip_protocol='tcp', from_port=-1,
parent_group_id='abc', ip_protocol='tcp', from_port=1,
to_port=2000, cidr='1.2.3.4/32', group_id='123'
)
@mock.patch.object(shade.OpenStackCloud, 'get_security_group')
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
def test_create_security_group_rule_nova_no_ports(self,
mock_nova, mock_get):
self.cloud.secgroup_source = 'nova'
new_rule = fakes.FakeNovaSecgroupRule(
id='xyz', from_port=1, to_port=65535, ip_protocol='tcp',
cidr='1.2.3.4/32')
mock_nova.security_group_rules.create.return_value = new_rule
mock_get.return_value = {'id': 'abc'}
self.cloud.create_security_group_rule(
'abc', protocol='tcp',
remote_ip_prefix='1.2.3.4/32', remote_group_id='123')
mock_nova.security_group_rules.create.assert_called_once_with(
parent_group_id='abc', ip_protocol='tcp', from_port=1,
to_port=65535, cidr='1.2.3.4/32', group_id='123'
)
@mock.patch.object(shade.OpenStackCloud, 'neutron_client')
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
def test_create_security_group_rule_none(self, mock_nova, mock_neutron):