Merge "Add network options to security group rule create"

5 files changed, 127 insertions, 15 deletions

diff --git a/doc/source/command-objects/security-group-rule.rst b/doc/source/command-objects/security-group-rule.rst
index fa07c377..9aa82cd3 100644
--- a/doc/source/command-objects/security-group-rule.rst
+++ b/doc/source/command-objects/security-group-rule.rst
@@ -16,6 +16,8 @@ Create a new security group rule
         [--proto <proto>]
         [--src-ip <ip-address> | --src-group <group>]
         [--dst-port <port-range>]
+        [--ingress | --egress]
+        [--ethertype <ethertype>]
         <group>
 
 .. option:: --proto <proto>
@@ -24,7 +26,8 @@ Create a new security group rule
 
 .. option:: --src-ip <ip-address>
 
-    Source IP address block (may use CIDR notation; default: 0.0.0.0/0)
+    Source IP address block
+    (may use CIDR notation; default for IPv4 rule: 0.0.0.0/0)
 
 .. option:: --src-group <group>
 
@@ -35,6 +38,24 @@ Create a new security group rule
     Destination port, may be a single port or port range: 137:139
     (only required for IP protocols tcp and udp)
 
+.. option:: --ingress
+
+    Rule applies to incoming network traffic (default)
+
+    *Network version 2 only*
+
+.. option:: --egress
+
+    Rule applies to outgoing network traffic
+
+    *Network version 2 only*
+
+.. option:: --ethertype <ethertype>
+
+    Ethertype of network traffic (IPv4, IPv6; default: IPv4)
+
+    *Network version 2 only*
+
 .. describe:: <group>
 
     Create rule in this security group (name or ID)
diff --git a/functional/tests/network/v2/test_security_group_rule.py b/functional/tests/network/v2/test_security_group_rule.py
index 9c0b66e8..26e6e0e4 100644
--- a/functional/tests/network/v2/test_security_group_rule.py
+++ b/functional/tests/network/v2/test_security_group_rule.py
@@ -38,6 +38,7 @@ class SecurityGroupRuleTests(test.TestCase):
         raw_output = cls.openstack('security group rule create ' +
                                    cls.SECURITY_GROUP_NAME +
                                    ' --proto tcp --dst-port 80:80' +
+                                   ' --ingress --ethertype IPv4' +
                                    opts)
         cls.SECURITY_GROUP_RULE_ID = raw_output.strip('\n')
 
diff --git a/openstackclient/network/v2/security_group_rule.py b/openstackclient/network/v2/security_group_rule.py
index c6fb3558..83250914 100644
--- a/openstackclient/network/v2/security_group_rule.py
+++ b/openstackclient/network/v2/security_group_rule.py
@@ -68,7 +68,9 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
             help='Create rule in this security group (name or ID)',
         )
         # TODO(rtheis): Add support for additional protocols for network.
-        # Until then, continue enforcing the compute choices.
+        # Until then, continue enforcing the compute choices. When additional
+        # protocols are added, the default ethertype must be determined
+        # based on the protocol.
         parser.add_argument(
             "--proto",
             metavar="<proto>",
@@ -81,9 +83,8 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
         source_group.add_argument(
             "--src-ip",
             metavar="<ip-address>",
-            default="0.0.0.0/0",
-            help="Source IP address block (may use CIDR notation; default: "
-                 "0.0.0.0/0)",
+            help="Source IP address block (may use CIDR notation; "
+                 "default for IPv4 rule: 0.0.0.0/0)",
         )
         source_group.add_argument(
             "--src-group",
@@ -100,6 +101,27 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
         )
         return parser
 
+    def update_parser_network(self, parser):
+        direction_group = parser.add_mutually_exclusive_group()
+        direction_group.add_argument(
+            '--ingress',
+            action='store_true',
+            help='Rule applies to incoming network traffic (default)',
+        )
+        direction_group.add_argument(
+            '--egress',
+            action='store_true',
+            help='Rule applies to outgoing network traffic',
+        )
+        parser.add_argument(
+            '--ethertype',
+            metavar='<ethertype>',
+            choices=['IPv4', 'IPv6'],
+            help='Ethertype of network traffic '
+                 '(IPv4, IPv6; default: IPv4)',
+        )
+        return parser
+
     def take_action_network(self, client, parsed_args):
         # Get the security group ID to hold the rule.
         security_group_id = client.find_security_group(
@@ -109,12 +131,18 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
 
         # Build the create attributes.
         attrs = {}
-        # TODO(rtheis): Add --direction option. Until then, continue
-        # with the default of 'ingress'.
-        attrs['direction'] = 'ingress'
-        # TODO(rtheis): Add --ethertype option. Until then, continue
-        # with the default of 'IPv4'
-        attrs['ethertype'] = 'IPv4'
+        # NOTE(rtheis): A direction must be specified and ingress
+        # is the default.
+        if parsed_args.ingress or not parsed_args.egress:
+            attrs['direction'] = 'ingress'
+        if parsed_args.egress:
+            attrs['direction'] = 'egress'
+        if parsed_args.ethertype:
+            attrs['ethertype'] = parsed_args.ethertype
+        else:
+            # NOTE(rtheis): Default based on protocol is IPv4 for now.
+            # Once IPv6 protocols are added, this will need to be updated.
+            attrs['ethertype'] = 'IPv4'
         # TODO(rtheis): Add port range support (type and code) for icmp
         # protocol. Until then, continue ignoring the port range.
         if parsed_args.proto != 'icmp':
@@ -126,8 +154,10 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
                 parsed_args.src_group,
                 ignore_missing=False
             ).id
-        else:
+        elif parsed_args.src_ip is not None:
             attrs['remote_ip_prefix'] = parsed_args.src_ip
+        elif attrs['ethertype'] == 'IPv4':
+            attrs['remote_ip_prefix'] = '0.0.0.0/0'
         attrs['security_group_id'] = security_group_id
 
         # Create and show the security group rule.
@@ -145,17 +175,22 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
             from_port, to_port = -1, -1
         else:
             from_port, to_port = parsed_args.dst_port
+        src_ip = None
         if parsed_args.src_group is not None:
             parsed_args.src_group = utils.find_resource(
                 client.security_groups,
                 parsed_args.src_group,
             ).id
+        if parsed_args.src_ip is not None:
+            src_ip = parsed_args.src_ip
+        else:
+            src_ip = '0.0.0.0/0'
         obj = client.security_group_rules.create(
             group.id,
             parsed_args.proto,
             from_port,
             to_port,
-            parsed_args.src_ip,
+            src_ip,
             parsed_args.src_group,
         )
         return _format_security_group_rule_show(obj._info)
diff --git a/openstackclient/tests/network/v2/test_security_group_rule.py b/openstackclient/tests/network/v2/test_security_group_rule.py
index 6aad8599..9e9fd120 100644
--- a/openstackclient/tests/network/v2/test_security_group_rule.py
+++ b/openstackclient/tests/network/v2/test_security_group_rule.py
@@ -97,7 +97,7 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
         self.assertRaises(tests_utils.ParserException,
                           self.check_parser, self.cmd, [], [])
 
-    def test_create_source_group_and_ip(self):
+    def test_create_all_source_options(self):
         arglist = [
             '--src-ip', '10.10.0.0/24',
             '--src-group', self._security_group.id,
@@ -114,6 +114,14 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
         self.assertRaises(tests_utils.ParserException,
                           self.check_parser, self.cmd, arglist, [])
 
+    def test_create_bad_ethertype(self):
+        arglist = [
+            '--ethertype', 'foo',
+            self._security_group.id,
+        ]
+        self.assertRaises(tests_utils.ParserException,
+                          self.check_parser, self.cmd, arglist, [])
+
     def test_create_default_rule(self):
         self._setup_security_group_rule({
             'port_range_max': 443,
@@ -124,6 +132,8 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
             self._security_group.id,
         ]
         verifylist = [
+            ('dst_port', (self._security_group_rule.port_range_min,
+                          self._security_group_rule.port_range_max)),
             ('group', self._security_group.id),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -150,12 +160,14 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
         })
         arglist = [
             '--dst-port', str(self._security_group_rule.port_range_min),
+            '--ingress',
             '--src-group', self._security_group.name,
             self._security_group.id,
         ]
         verifylist = [
             ('dst_port', (self._security_group_rule.port_range_min,
                           self._security_group_rule.port_range_max)),
+            ('ingress', True),
             ('src_group', self._security_group.name),
             ('group', self._security_group.id),
         ]
@@ -206,6 +218,43 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
         self.assertEqual(tuple(self.expected_columns), columns)
         self.assertEqual(self.expected_data, data)
 
+    def test_create_network_options(self):
+        self._setup_security_group_rule({
+            'direction': 'egress',
+            'ethertype': 'IPv6',
+            'port_range_max': 443,
+            'port_range_min': 443,
+            'remote_group_id': None,
+            'remote_ip_prefix': None,
+        })
+        arglist = [
+            '--dst-port', str(self._security_group_rule.port_range_min),
+            '--egress',
+            '--ethertype', self._security_group_rule.ethertype,
+            self._security_group.id,
+        ]
+        verifylist = [
+            ('dst_port', (self._security_group_rule.port_range_min,
+                          self._security_group_rule.port_range_max)),
+            ('egress', True),
+            ('ethertype', self._security_group_rule.ethertype),
+            ('group', self._security_group.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.network.create_security_group_rule.assert_called_once_with(**{
+            'direction': self._security_group_rule.direction,
+            'ethertype': self._security_group_rule.ethertype,
+            'port_range_max': self._security_group_rule.port_range_max,
+            'port_range_min': self._security_group_rule.port_range_min,
+            'protocol': self._security_group_rule.protocol,
+            'security_group_id': self._security_group.id,
+        })
+        self.assertEqual(tuple(self.expected_columns), columns)
+        self.assertEqual(self.expected_data, data)
+
 
 class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
 
@@ -241,7 +290,7 @@ class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
         self.assertRaises(tests_utils.ParserException,
                           self.check_parser, self.cmd, [], [])
 
-    def test_create_source_group_and_ip(self):
+    def test_create_all_source_options(self):
         arglist = [
             '--src-ip', '10.10.0.0/24',
             '--src-group', self._security_group.id,
diff --git a/releasenotes/notes/bug-1519512-48d98f09e44220a3.yaml b/releasenotes/notes/bug-1519512-48d98f09e44220a3.yaml
new file mode 100644
index 00000000..1d275c57
--- /dev/null
+++ b/releasenotes/notes/bug-1519512-48d98f09e44220a3.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - Add ``--ingress``, ``--egress``, and ``--ethertype`` options to the
+    ``security group rule create`` command for Network v2 only. These
+    options enable ``egress`` and ``IPv6`` security group rules.
+    [Bug `1519512 <https://bugs.launchpad.net/bugs/1519512>`_]