From be7a75814ca4a503dd384f36166f93f12f6cb8da Mon Sep 17 00:00:00 2001
From: Doug Wiegley <dwiegley@salesforce.com>
Date: Wed, 13 Feb 2019 14:17:33 -0700
Subject: [PATCH] Add 'security_group' type support to network rbac commands

Partial-Bug: #1817119
Depends-On: https://review.openstack.org/635311
Change-Id: I5f132fa54714514d8dae62df8bc494f3f6476768
---
 .../cli/command-objects/network-rbac.rst      |  4 +-
 openstackclient/network/v2/network_rbac.py    | 13 ++++--
 .../tests/unit/network/v2/fakes.py            | 33 +++++++++++++++
 .../unit/network/v2/test_network_rbac.py      | 40 +++++++++++++++++++
 ...c-add-security-group-35370701a06ac906.yaml |  5 +++
 5 files changed, 89 insertions(+), 6 deletions(-)
 create mode 100644 releasenotes/notes/rbac-add-security-group-35370701a06ac906.yaml

diff --git a/doc/source/cli/command-objects/network-rbac.rst b/doc/source/cli/command-objects/network-rbac.rst
index 45fd354deb..22733a2cd2 100644
--- a/doc/source/cli/command-objects/network-rbac.rst
+++ b/doc/source/cli/command-objects/network-rbac.rst
@@ -26,7 +26,7 @@ Create network RBAC policy
 
 .. option:: --type <type>
 
-    Type of the object that RBAC policy affects ("qos_policy" or "network") (required)
+    Type of the object that RBAC policy affects ("security_group", "qos_policy" or "network") (required)
 
 .. option:: --action <action>
 
@@ -90,7 +90,7 @@ List network RBAC policies
 
 .. option:: --type <type>
 
-    List network RBAC policies according to given object type ("qos_policy" or "network")
+    List network RBAC policies according to given object type ("security_group", "qos_policy" or "network")
 
 .. option:: --action <action>
 
diff --git a/openstackclient/network/v2/network_rbac.py b/openstackclient/network/v2/network_rbac.py
index 6cf82559dd..140c837e38 100644
--- a/openstackclient/network/v2/network_rbac.py
+++ b/openstackclient/network/v2/network_rbac.py
@@ -48,6 +48,10 @@ def _get_attrs(client_manager, parsed_args):
         object_id = network_client.find_qos_policy(
             parsed_args.rbac_object,
             ignore_missing=False).id
+    if parsed_args.type == 'security_group':
+        object_id = network_client.find_security_group(
+            parsed_args.rbac_object,
+            ignore_missing=False).id
     attrs['object_id'] = object_id
 
     identity_client = client_manager.identity
@@ -87,9 +91,9 @@ class CreateNetworkRBAC(command.ShowOne):
             '--type',
             metavar="<type>",
             required=True,
-            choices=['qos_policy', 'network'],
+            choices=['security_group', 'qos_policy', 'network'],
             help=_('Type of the object that RBAC policy '
-                   'affects ("qos_policy" or "network")')
+                   'affects ("security_group", "qos_policy" or "network")')
         )
         parser.add_argument(
             '--action',
@@ -178,9 +182,10 @@ class ListNetworkRBAC(command.Lister):
         parser.add_argument(
             '--type',
             metavar='<type>',
-            choices=['qos_policy', 'network'],
+            choices=['security_group', 'qos_policy', 'network'],
             help=_('List network RBAC policies according to '
-                   'given object type ("qos_policy" or "network")')
+                   'given object type ("security_group", "qos_policy" '
+                   'or "network")')
         )
         parser.add_argument(
             '--action',
diff --git a/openstackclient/tests/unit/network/v2/fakes.py b/openstackclient/tests/unit/network/v2/fakes.py
index 28e92d1196..5860ff452f 100644
--- a/openstackclient/tests/unit/network/v2/fakes.py
+++ b/openstackclient/tests/unit/network/v2/fakes.py
@@ -908,6 +908,39 @@ class FakeNetworkQosPolicy(object):
         return mock.Mock(side_effect=qos_policies)
 
 
+class FakeNetworkSecGroup(object):
+    """Fake one security group."""
+
+    @staticmethod
+    def create_one_security_group(attrs=None):
+        """Create a fake security group.
+
+        :param Dictionary attrs:
+            A dictionary with all attributes
+        :return:
+            A FakeResource object with name, id, etc.
+        """
+        attrs = attrs or {}
+        sg_id = attrs.get('id') or 'security-group-id-' + uuid.uuid4().hex
+
+        # Set default attributes.
+        security_group_attrs = {
+            'name': 'security-group-name-' + uuid.uuid4().hex,
+            'id': sg_id,
+            'tenant_id': 'project-id-' + uuid.uuid4().hex,
+            'description': 'security-group-description-' + uuid.uuid4().hex
+        }
+
+        security_group = fakes.FakeResource(
+            info=copy.deepcopy(security_group_attrs),
+            loaded=True)
+
+        # Set attributes with special mapping in OpenStack SDK.
+        security_group.project_id = security_group_attrs['tenant_id']
+
+        return security_group
+
+
 class FakeNetworkQosRule(object):
     """Fake one or more Network QoS rules."""
 
diff --git a/openstackclient/tests/unit/network/v2/test_network_rbac.py b/openstackclient/tests/unit/network/v2/test_network_rbac.py
index 70c3852868..96440091c1 100644
--- a/openstackclient/tests/unit/network/v2/test_network_rbac.py
+++ b/openstackclient/tests/unit/network/v2/test_network_rbac.py
@@ -37,6 +37,7 @@ class TestCreateNetworkRBAC(TestNetworkRBAC):
 
     network_object = network_fakes.FakeNetwork.create_one_network()
     qos_object = network_fakes.FakeNetworkQosPolicy.create_one_qos_policy()
+    sg_object = network_fakes.FakeNetworkSecGroup.create_one_security_group()
     project = identity_fakes_v3.FakeProject.create_one_project()
     rbac_policy = network_fakes.FakeNetworkRBAC.create_one_network_rbac(
         attrs={'tenant_id': project.id,
@@ -74,6 +75,8 @@ class TestCreateNetworkRBAC(TestNetworkRBAC):
             return_value=self.network_object)
         self.network.find_qos_policy = mock.Mock(
             return_value=self.qos_object)
+        self.network.find_security_group = mock.Mock(
+            return_value=self.sg_object)
         self.projects_mock.get.return_value = self.project
 
     def test_network_rbac_create_no_type(self):
@@ -258,6 +261,43 @@ class TestCreateNetworkRBAC(TestNetworkRBAC):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, list(data))
 
+    def test_network_rbac_create_security_group_object(self):
+        self.rbac_policy.object_type = 'security_group'
+        self.rbac_policy.object_id = self.sg_object.id
+        arglist = [
+            '--type', 'security_group',
+            '--action', self.rbac_policy.action,
+            '--target-project', self.rbac_policy.target_tenant,
+            self.sg_object.name,
+        ]
+        verifylist = [
+            ('type', 'security_group'),
+            ('action', self.rbac_policy.action),
+            ('target_project', self.rbac_policy.target_tenant),
+            ('rbac_object', self.sg_object.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.network.create_rbac_policy.assert_called_with(**{
+            'object_id': self.sg_object.id,
+            'object_type': 'security_group',
+            'action': self.rbac_policy.action,
+            'target_tenant': self.rbac_policy.target_tenant,
+        })
+        self.data = [
+            self.rbac_policy.action,
+            self.rbac_policy.id,
+            self.sg_object.id,
+            'security_group',
+            self.rbac_policy.tenant_id,
+            self.rbac_policy.target_tenant,
+        ]
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.data, list(data))
+
 
 class TestDeleteNetworkRBAC(TestNetworkRBAC):
 
diff --git a/releasenotes/notes/rbac-add-security-group-35370701a06ac906.yaml b/releasenotes/notes/rbac-add-security-group-35370701a06ac906.yaml
new file mode 100644
index 0000000000..f21eebb795
--- /dev/null
+++ b/releasenotes/notes/rbac-add-security-group-35370701a06ac906.yaml
@@ -0,0 +1,5 @@
+features:
+  - |
+    Add ``security_group`` as a valid ``--type`` value for the
+    ``network rbac create`` and ``network rbac list`` commands.
+