[log] Add 'firewall_group' as a loggable resource type for logging

Currently, OSC plugin for logging only supports security group as
a loggable resource. This patch aims to add 'firewall_group' as a
supported resource type.

Co-Authored-By: Van Hung Pham <HungPV@vn.fujitsu.com>
Depends-On: Ie10063197f02679e987e87cb4852f5230a02f76d
Change-Id: I2ac92004c2ad3769c9749e131718a8ef0003c4bd
Partial-Bug: #1720727

3 files changed, 126 insertions, 16 deletions

diff --git a/neutronclient/osc/v2/logging/network_log.py b/neutronclient/osc/v2/logging/network_log.py
index 534e48d..1946a40 100644
--- a/neutronclient/osc/v2/logging/network_log.py
+++ b/neutronclient/osc/v2/logging/network_log.py
@@ -1,4 +1,4 @@
-# Copyright 2017 FUJTISU LIMITED.
+# Copyright 2017-2018 FUJTISU LIMITED.
 # All Rights Reserved
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -25,7 +25,7 @@ from oslo_log import log as logging
 from neutronclient._i18n import _
 from neutronclient.common import utils as nc_utils
 from neutronclient.osc import utils as osc_utils
-
+from neutronclient.osc.v2.fwaas import constants as fwaas_const
 
 LOG = logging.getLogger(__name__)
 
@@ -77,17 +77,23 @@ def _get_common_attrs(client_manager, parsed_args, is_create=True):
                 parsed_args.project,
                 parsed_args.project_domain,
             ).id
+        resource_type = parsed_args.resource_type
+        attrs['resource_type'] = resource_type
         if parsed_args.resource:
+            cmd_resource = None
+            if resource_type == fwaas_const.FWG:
+                cmd_resource = fwaas_const.CMD_FWG
             attrs['resource_id'] = client.find_resource(
-                'security_group', parsed_args.resource)['id']
+                resource_type,
+                parsed_args.resource,
+                cmd_resource=cmd_resource)['id']
+
         if parsed_args.target:
             # NOTE(yushiro) Currently, we're supporting only port
             attrs['target_id'] = client.find_resource(
                 'port', parsed_args.target)['id']
         if parsed_args.event:
             attrs['event'] = parsed_args.event
-        if parsed_args.resource_type:
-            attrs['resource_type'] = parsed_args.resource_type
     if parsed_args.enable:
         attrs['enabled'] = True
     if parsed_args.disable:
@@ -117,8 +123,8 @@ class CreateNetworkLog(command.ShowOne):
             type=nc_utils.convert_to_uppercase,
             help=_('An event to store with log'))
         # NOTE(yushiro) '--resource-type' is managed by following command:
-        # "openstack network loggable resource list". Therefore, this option
-        # shouldn't have "choices" like ['security_group']
+        # "openstack network loggable resources list". Therefore, this option
+        # shouldn't have "choices" like ['security_group', 'firewall_group']
         parser.add_argument(
             '--resource-type',
             metavar='<resource-type>',
@@ -126,12 +132,13 @@ class CreateNetworkLog(command.ShowOne):
             type=nc_utils.convert_to_lowercase,
             help=_('Network log type(s). '
                    'You can see supported type(s) with following command:\n'
-                   '$ openstack network loggable resource list'))
+                   '$ openstack network loggable resources list'))
         parser.add_argument(
             '--resource',
             metavar='<resource>',
-            help=_('Security group (name or ID) for logging. You can control '
-                   'for logging target combination with --target option.'))
+            help=_('Name or ID of resource (security group or firewall group) '
+                   'that used for logging. You can control for logging target '
+                   'combination with --target option.'))
         parser.add_argument(
             '--target',
             metavar='<target>',
@@ -171,13 +178,13 @@ class DeleteNetworkLog(command.Command):
             except Exception as e:
                 result += 1
                 LOG.error(_("Failed to delete network log with "
-                          "name or ID '%(network_log)s': %(e)s"),
+                            "name or ID '%(network_log)s': %(e)s"),
                           {'network_log': log_res, 'e': e})
 
         if result > 0:
             total = len(parsed_args.network_log)
             msg = (_("%(result)s of %(total)s network log(s) "
-                   "failed to delete") % {'result': result, 'total': total})
+                     "failed to delete") % {'result': result, 'total': total})
             raise exceptions.CommandError(msg)
 
 
@@ -221,10 +228,17 @@ class ListNetworkLog(command.Lister):
             if d['event']:
                 event = e_prefix + d['event'].upper()
             port = '(port) ' + d['target_id'] if d['target_id'] else ''
-            sg = ('(security_group) ' + d['resource_id']
-                  if d['resource_id'] else '')
+            resource_type = d['resource_type']
+            if d['resource_id']:
+                res = '(%s) %s' % (resource_type, d['resource_id'])
+            else:
+                res = ''
             t_prefix = 'Logged: '
-            t = sg + ' on ' + port if port and sg else sg + port
+            if port and res:
+                t = '%s on %s' % (res, port)
+            else:
+                # Either of res and port is empty, so concatenation works fine
+                t = res + port
             target = t_prefix + t if t else t_prefix + '(None specified)'
             d['summary'] = ',\n'.join([event, target])
         return ext_data
diff --git a/neutronclient/tests/unit/osc/v2/logging/test_network_log.py b/neutronclient/tests/unit/osc/v2/logging/test_network_log.py
index bceaf32..43c0bcb 100644
--- a/neutronclient/tests/unit/osc/v2/logging/test_network_log.py
+++ b/neutronclient/tests/unit/osc/v2/logging/test_network_log.py
@@ -1,4 +1,4 @@
-# Copyright 2017 FUJITSU LIMITED
+# Copyright 2017-2018 FUJITSU LIMITED
 # All Rights Reserved
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -29,6 +29,7 @@ from neutronclient.tests.unit.osc.v2.logging import fakes
 
 _log = fakes.NetworkLog().create()
 RES_TYPE_SG = 'security_group'
+RES_TYPE_FWG = 'firewall_group'
 CONVERT_MAP = {
     'project': 'project_id',
     'enable': 'enabled',
@@ -137,6 +138,12 @@ class TestCreateNetworkLog(TestNetworkLog):
             return_value={'log': _log})
         self.mocked = self.neutronclient.create_network_log
         self.cmd = network_log.CreateNetworkLog(self.app, self.namespace)
+        loggables = {
+            "loggable_resources": [{"type": RES_TYPE_SG,
+                                    "type": RES_TYPE_FWG}]
+        }
+        self.neutronclient.list_network_loggable_resources = mock.Mock(
+            return_value=loggables)
 
     def _update_expect_response(self, request, response):
         """Set expected request and response
@@ -279,6 +286,90 @@ class TestCreateNetworkLog(TestNetworkLog):
                 testtools.matchers._impl.MismatchError,
                 self.check_parser, self.cmd, arglist, verifylist)
 
+    def test_create_with_valid_fwg_resource(self):
+        name = self.res['name']
+        resource_id = 'valid_fwg_id'
+        resource_type = RES_TYPE_FWG
+        # Test with valid FWG ID
+        with mock.patch.object(self.neutronclient, 'find_resource',
+                               return_value={'id': resource_id}):
+            arglist = [name,
+                       '--resource-type', resource_type,
+                       '--resource', resource_id
+                       ]
+            verifylist = [
+                ('name', name),
+                ('resource_type', resource_type),
+                ('resource', resource_id)
+            ]
+
+            parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+            headers, data = self.cmd.take_action(parsed_args)
+            expect = {
+                'name': self.res['name'],
+                'resource_type': RES_TYPE_FWG,
+                'resource_id': 'valid_fwg_id',
+            }
+            self.neutronclient.find_resource.assert_called_with(
+                resource_type,
+                resource_id,
+                cmd_resource='fwaas_firewall_group')
+            self.mocked.assert_called_once_with({'log': expect})
+            self.assertEqual(self.ordered_headers, headers)
+            self.assertEqual(self.ordered_data, data)
+
+    def test_create_with_invalid_fwg_resource(self):
+        name = self.res['name']
+        resource_id = 'invalid_fwg_id'
+        resource_type = RES_TYPE_FWG
+        # Test with invalid FWG ID
+        with mock.patch.object(self.neutronclient, 'find_resource',
+                               side_effect=exceptions.NotFound(code=0)):
+            arglist = [name,
+                       '--resource-type', resource_type,
+                       '--resource', resource_id
+                       ]
+            verifylist = [
+                ('name', name),
+                ('resource_type', resource_type),
+                ('resource', resource_id)
+            ]
+
+            parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+            self.assertRaises(exceptions.NotFound,
+                              self.cmd.take_action,
+                              parsed_args)
+            self.neutronclient.find_resource.assert_called_with(
+                resource_type,
+                resource_id,
+                cmd_resource='fwaas_firewall_group')
+            self.mocked.assert_not_called()
+
+    def test_create_with_invalid_resource_type(self):
+        name = self.res['name']
+        resource_type = 'invalid_resource_type'
+        resource_id = 'valid_fwg_id'
+        with mock.patch.object(self.neutronclient, 'find_resource',
+                               side_effect=exceptions.NotFound(code=0)):
+            arglist = [name,
+                       '--resource-type', resource_type,
+                       '--resource', resource_id
+                       ]
+            verifylist = [
+                ('name', name),
+                ('resource_type', resource_type),
+                ('resource', resource_id)
+            ]
+            parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+            self.assertRaises(exceptions.NotFound,
+                              self.cmd.take_action,
+                              parsed_args)
+            self.neutronclient.find_resource.assert_called_with(
+                resource_type,
+                resource_id,
+                cmd_resource=None)
+            self.mocked.assert_not_called()
+
 
 class TestListNetworkLog(TestNetworkLog):
 
diff --git a/releasenotes/notes/support-firewall-group-resource-type-5ad1b69cabcb4aa6.yaml b/releasenotes/notes/support-firewall-group-resource-type-5ad1b69cabcb4aa6.yaml
new file mode 100644
index 0000000..78fbff8
--- /dev/null
+++ b/releasenotes/notes/support-firewall-group-resource-type-5ad1b69cabcb4aa6.yaml
@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    CLI support for the "firewal_group" as a loggable resource type for logging
+    feature, which is enhanced FWaaS functionality, as OSC plugin commands.