[fwaas] Can now create disabled firewall rules

Previously, the --disabled flag was ignored when creating
firewall rules. This patch fixes that problem, closes the bug
and adds a test. I changed update_dict so that values are sent
on to the API call if they are not None (So False values are
sent as well).

Before this patch, when creating a resource, values were
inserted into the API call only if their value was not None
or False. This worked fine for values whose default value
was False. 'enabled', however, has a default value of True.
That means that sending enable=False wouldn't work.

After the change to update_dict the issue, however, was that
'shared' and 'audited' were then always sent (Even if their
default was False) but the unit tests weren't expecting a False
value since it wasn't explicitly stated. Instead of changing
the unit tests, the solution is to only send the value in the
first place if it is not the default value, using argparse's
default mechanism. If it is the default, then it is written as
None and so is not sent, and not checked by the unit tests either.

Closes-Bug: 1250028
Change-Id: I142089ffd612fb52a326e8951a385920af852a2c

7 files changed, 36 insertions, 18 deletions

diff --git a/neutronclient/neutron/v2_0/__init__.py b/neutronclient/neutron/v2_0/__init__.py
index 3c995d6..a30fcf3 100644
--- a/neutronclient/neutron/v2_0/__init__.py
+++ b/neutronclient/neutron/v2_0/__init__.py
@@ -297,8 +297,14 @@ def _merge_args(qCmd, parsed_args, _extra_values, value_specs):
 
 
 def update_dict(obj, dict, attributes):
+    """Update dict with fields from obj.attributes
+
+    :param obj: the object updated into dict
+    :param dict: the result dictionary
+    :param attributes: a list of attributes belonging to obj
+    """
     for attribute in attributes:
-        if hasattr(obj, attribute) and getattr(obj, attribute):
+        if hasattr(obj, attribute) and getattr(obj, attribute) is not None:
             dict[attribute] = getattr(obj, attribute)
 
 
diff --git a/neutronclient/neutron/v2_0/fw/firewall.py b/neutronclient/neutron/v2_0/fw/firewall.py
index 589053f..3a532ba 100644
--- a/neutronclient/neutron/v2_0/fw/firewall.py
+++ b/neutronclient/neutron/v2_0/fw/firewall.py
@@ -17,6 +17,7 @@
 #
 # vim: tabstop=4 shiftwidth=4 softtabstop=4
 
+import argparse
 import logging
 
 from neutronclient.neutron import v2_0 as neutronv20
@@ -59,7 +60,8 @@ class CreateFirewall(neutronv20.CreateCommand):
         parser.add_argument(
             '--shared',
             action='store_true',
-            help='set shared to True (default False)')
+            help='set shared to True (default False)',
+            default=argparse.SUPPRESS)
         parser.add_argument(
             '--admin-state-down',
             dest='admin_state',
diff --git a/neutronclient/neutron/v2_0/fw/firewallpolicy.py b/neutronclient/neutron/v2_0/fw/firewallpolicy.py
index f5b95ba..ac99d91 100644
--- a/neutronclient/neutron/v2_0/fw/firewallpolicy.py
+++ b/neutronclient/neutron/v2_0/fw/firewallpolicy.py
@@ -17,6 +17,7 @@
 #
 # vim: tabstop=4 shiftwidth=4 softtabstop=4
 
+import argparse
 import logging
 import string
 
@@ -70,7 +71,8 @@ class CreateFirewallPolicy(neutronv20.CreateCommand):
             '--shared',
             dest='shared',
             action='store_true',
-            help='to create a shared policy')
+            help='to create a shared policy',
+            default=argparse.SUPPRESS)
         parser.add_argument(
             '--firewall-rules', type=string.split,
             help='ordered list of whitespace-delimited firewall rule '
@@ -78,7 +80,8 @@ class CreateFirewallPolicy(neutronv20.CreateCommand):
         parser.add_argument(
             '--audited',
             action='store_true',
-            help='to set audited to True')
+            help='to set audited to True',
+            default=argparse.SUPPRESS)
 
     def args2body(self, parsed_args):
         if parsed_args.firewall_rules:
diff --git a/neutronclient/neutron/v2_0/fw/firewallrule.py b/neutronclient/neutron/v2_0/fw/firewallrule.py
index 9458122..d158332 100644
--- a/neutronclient/neutron/v2_0/fw/firewallrule.py
+++ b/neutronclient/neutron/v2_0/fw/firewallrule.py
@@ -17,6 +17,7 @@
 #
 # vim: tabstop=4 shiftwidth=4 softtabstop=4
 
+import argparse
 import logging
 
 from neutronclient.neutron import v2_0 as neutronv20
@@ -83,7 +84,8 @@ class CreateFirewallRule(neutronv20.CreateCommand):
             '--shared',
             dest='shared',
             action='store_true',
-            help='set shared to True (default False)')
+            help='set shared to True (default False)',
+            default=argparse.SUPPRESS)
         parser.add_argument(
             '--source-ip-address',
             help='source ip address or subnet')
@@ -100,7 +102,8 @@ class CreateFirewallRule(neutronv20.CreateCommand):
             '--disabled',
             dest='enabled',
             action='store_false',
-            help='to disable this rule')
+            help='to disable this rule',
+            default=argparse.SUPPRESS)
         parser.add_argument(
             '--protocol', choices=['tcp', 'udp', 'icmp', 'any'],
             required=True,
diff --git a/neutronclient/neutron/v2_0/network.py b/neutronclient/neutron/v2_0/network.py
index c4bcea3..40be9b3 100644
--- a/neutronclient/neutron/v2_0/network.py
+++ b/neutronclient/neutron/v2_0/network.py
@@ -122,7 +122,8 @@ class CreateNetwork(neutronV20.CreateCommand):
         parser.add_argument(
             '--shared',
             action='store_true',
-            help='Set the network as shared')
+            help='Set the network as shared',
+            default=argparse.SUPPRESS)
         parser.add_argument(
             'name', metavar='NAME',
             help='Name of network to create')
@@ -131,10 +132,8 @@ class CreateNetwork(neutronV20.CreateCommand):
         body = {'network': {
             'name': parsed_args.name,
             'admin_state_up': parsed_args.admin_state}, }
-        if parsed_args.tenant_id:
-            body['network'].update({'tenant_id': parsed_args.tenant_id})
-        if parsed_args.shared:
-            body['network'].update({'shared': parsed_args.shared})
+        neutronV20.update_dict(parsed_args, body['network'],
+                               ['shared', 'tenant_id'])
         return body
 
 
diff --git a/neutronclient/tests/unit/fw/test_cli20_firewallrule.py b/neutronclient/tests/unit/fw/test_cli20_firewallrule.py
index 2c9ded9..a6aceba 100644
--- a/neutronclient/tests/unit/fw/test_cli20_firewallrule.py
+++ b/neutronclient/tests/unit/fw/test_cli20_firewallrule.py
@@ -25,7 +25,7 @@ from neutronclient.tests.unit import test_cli20
 
 class CLITestV20FirewallRuleJSON(test_cli20.CLITestV20Base):
 
-    def test_create_firewall_rule_with_mandatory_params(self):
+    def _test_create_firewall_rule_with_mandatory_params(self, enabled):
         """firewall-rule-create with mandatory (none) params only."""
         resource = 'firewall_rule'
         cmd = firewallrule.CreateFirewallRule(test_cli20.MyApp(sys.stdout),
@@ -35,17 +35,24 @@ class CLITestV20FirewallRuleJSON(test_cli20.CLITestV20Base):
         my_id = 'myid'
         protocol = 'tcp'
         action = 'allow'
+        enabled_flag = '--enabled' if enabled else '--disabled'
         args = ['--tenant-id', tenant_id,
                 '--admin-state-up',
                 '--protocol', protocol,
                 '--action', action,
-                '--enabled']
+                enabled_flag]
         position_names = []
         position_values = []
         self._test_create_resource(resource, cmd, name, my_id, args,
                                    position_names, position_values,
                                    protocol=protocol, action=action,
-                                   enabled=True, tenant_id=tenant_id)
+                                   enabled=enabled, tenant_id=tenant_id)
+
+    def test_create_enabled_firewall_rule_with_mandatory_params(self):
+        self._test_create_firewall_rule_with_mandatory_params(enabled=True)
+
+    def test_create_disabled_firewall_rule_with_mandatory_params(self):
+        self._test_create_firewall_rule_with_mandatory_params(enabled=False)
 
     def _setup_create_firewall_rule_with_all_params(self, protocol='tcp'):
         """firewall-rule-create with all params set."""
diff --git a/neutronclient/tests/unit/test_cli20.py b/neutronclient/tests/unit/test_cli20.py
index 7c81976..e9ea5e1 100644
--- a/neutronclient/tests/unit/test_cli20.py
+++ b/neutronclient/tests/unit/test_cli20.py
@@ -180,8 +180,8 @@ class CLITestV20Base(testtools.TestCase):
     def _test_create_resource(self, resource, cmd,
                               name, myid, args,
                               position_names, position_values, tenant_id=None,
-                              tags=None, admin_state_up=True, shared=False,
-                              extra_body=None, **kwargs):
+                              tags=None, admin_state_up=True, extra_body=None,
+                              **kwargs):
         self.mox.StubOutWithMock(cmd, "get_client")
         self.mox.StubOutWithMock(self.client.httpclient, "request")
         cmd.get_client().MultipleTimes().AndReturn(self.client)
@@ -199,8 +199,6 @@ class CLITestV20Base(testtools.TestCase):
             body[resource].update({'tenant_id': tenant_id})
         if tags:
             body[resource].update({'tags': tags})
-        if shared:
-            body[resource].update({'shared': shared})
         if extra_body:
             body[resource].update(extra_body)
         body[resource].update(kwargs)