[fwaas] Don't ignore missing fw related resources

Previously when fwaas osc plugin was going to look for firewall related
resources, like e.g. asking neutron for firewall group or policy was
done with ignore_missing=True which means that openstack SDK client
didn't raise exception when requested object was not found on the
server.
That led to the weird and unclear error message displayed by the
OpenStack client which said something about "'NoneType' object is not
subscriptable".

This patch adds "ignore_missing=True" to all those "find_*" methods used
in the fwaas osc plugin. That way proper exception is raised by the SDK
client and valid error message is displayed to the user by OSC.

Closes-bug: #2142458

Change-Id: I309a5dbf61c65d5837d4ea2b3235aa41269ae73d
Signed-off-by: Slawek Kaplonski <skaplons@redhat.com>

neutronclient/osc/v2/fwaas/firewallgroup.py

@@ -129,19 +129,19 @@ def _get_common_attrs(client_manager, parsed_args, is_create=True):
     if (parsed_args.ingress_firewall_policy and
             parsed_args.no_ingress_firewall_policy):
         attrs['ingress_firewall_policy_id'] = client.find_firewall_policy(
-            parsed_args.ingress_firewall_policy)['id']
+            parsed_args.ingress_firewall_policy, ignore_missing=False)['id']
     elif parsed_args.ingress_firewall_policy:
         attrs['ingress_firewall_policy_id'] = client.find_firewall_policy(
-            parsed_args.ingress_firewall_policy)['id']
+            parsed_args.ingress_firewall_policy, ignore_missing=False)['id']
     elif parsed_args.no_ingress_firewall_policy:
         attrs['ingress_firewall_policy_id'] = None
     if (parsed_args.egress_firewall_policy and
             parsed_args.no_egress_firewall_policy):
         attrs['egress_firewall_policy_id'] = client.find_firewall_policy(
-            parsed_args.egress_firewall_policy)['id']
+            parsed_args.egress_firewall_policy, ignore_missing=False)['id']
     elif parsed_args.egress_firewall_policy:
         attrs['egress_firewall_policy_id'] = client.find_firewall_policy(
-            parsed_args.egress_firewall_policy)['id']
+            parsed_args.egress_firewall_policy, ignore_missing=False)['id']
     elif parsed_args.no_egress_firewall_policy:
         attrs['egress_firewall_policy_id'] = None
     if parsed_args.share:
@@ -165,7 +165,7 @@ def _get_common_attrs(client_manager, parsed_args, is_create=True):
             ports.append(client.find_port(p)['id'])
         if not is_create:
             ports += client.find_firewall_group(
-                parsed_args.firewall_group)['ports']
+                parsed_args.firewall_group, ignore_missing=False)['ports']
         attrs['ports'] = sorted(set(ports))
     elif parsed_args.no_port:
         attrs['ports'] = []
@@ -220,7 +220,8 @@ class DeleteFirewallGroup(command.Command):
         result = 0
         for fwg in parsed_args.firewall_group:
             try:
-                fwg_id = client.find_firewall_group(fwg)['id']
+                fwg_id = client.find_firewall_group(
+                    fwg, ignore_missing=False)['id']
                 client.delete_firewall_group(fwg_id)
             except Exception as e:
                 result += 1
@@ -281,7 +282,8 @@ class SetFirewallGroup(command.Command):
 
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
-        fwg_id = client.find_firewall_group(parsed_args.firewall_group)['id']
+        fwg_id = client.find_firewall_group(
+            parsed_args.firewall_group, ignore_missing=False)['id']
         attrs = _get_common_attrs(self.app.client_manager, parsed_args,
                                   is_create=False)
         try:
@@ -305,7 +307,8 @@ class ShowFirewallGroup(command.ShowOne):
 
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
-        fwg_id = client.find_firewall_group(parsed_args.firewall_group)['id']
+        fwg_id = client.find_firewall_group(
+            parsed_args.firewall_group, ignore_missing=False)['id']
         obj = client.get_firewall_group(fwg_id)
         display_columns, columns = utils.get_osc_show_columns_for_sdk_resource(
             obj, _attr_map_dict, ['location', 'tenant_id'])
@@ -366,7 +369,7 @@ class UnsetFirewallGroup(command.Command):
             attrs['admin_state_up'] = False
         if parsed_args.port:
             old = client.find_firewall_group(
-                parsed_args.firewall_group)['ports']
+                parsed_args.firewall_group, ignore_missing=False)['ports']
             new = [client.find_port(r)['id'] for r in parsed_args.port]
             attrs['ports'] = sorted(list(set(old) - set(new)))
         if parsed_args.all_port:
@@ -375,7 +378,8 @@ class UnsetFirewallGroup(command.Command):
 
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
-        fwg_id = client.find_firewall_group(parsed_args.firewall_group)['id']
+        fwg_id = client.find_firewall_group(
+            parsed_args.firewall_group, ignore_missing=False)['id']
         attrs = self._get_attrs(client, parsed_args)
         try:
             client.update_firewall_group(fwg_id, **attrs)

neutronclient/osc/v2/fwaas/firewallpolicy.py

@@ -67,16 +67,18 @@ def _get_common_attrs(client_manager, parsed_args, is_create=True):
     if parsed_args.firewall_rule and parsed_args.no_firewall_rule:
         _firewall_rules = []
         for f in parsed_args.firewall_rule:
-            _firewall_rules.append(client.find_firewall_rule(f)['id'])
+            _firewall_rules.append(
+                client.find_firewall_rule(f, ignore_missing=False)['id'])
         attrs[const.FWRS] = _firewall_rules
     elif parsed_args.firewall_rule:
         rules = []
         if not is_create:
             foobar = client.find_firewall_policy(
-                parsed_args.firewall_policy)
+                parsed_args.firewall_policy, ignore_missing=False)
             rules += foobar[const.FWRS]
         for f in parsed_args.firewall_rule:
-            rules.append(client.find_firewall_rule(f)['id'])
+            rules.append(
+                client.find_firewall_rule(f, ignore_missing=False)['id'])
         attrs[const.FWRS] = rules
     elif parsed_args.no_firewall_rule:
         attrs[const.FWRS] = []
@@ -173,7 +175,8 @@ class DeleteFirewallPolicy(command.Command):
         result = 0
         for fwp in parsed_args.firewall_policy:
             try:
-                fwp_id = client.find_firewall_policy(fwp)['id']
+                fwp_id = client.find_firewall_policy(
+                    fwp, ignore_missing=False)['id']
                 client.delete_firewall_policy(fwp_id)
             except Exception as e:
                 result += 1
@@ -220,12 +223,12 @@ class FirewallPolicyInsertRule(command.Command):
         if 'insert_before' in parsed_args:
             if parsed_args.insert_before:
                 _insert_before = client.find_firewall_rule(
-                    parsed_args.insert_before)['id']
+                    parsed_args.insert_before, ignore_missing=False)['id']
         _insert_after = ''
         if 'insert_after' in parsed_args:
             if parsed_args.insert_after:
                 _insert_after = client.find_firewall_rule(
-                    parsed_args.insert_after)['id']
+                    parsed_args.insert_after, ignore_missing=False)['id']
         return {'firewall_rule_id': _rule_id,
                 'insert_before': _insert_before,
                 'insert_after': _insert_after}
@@ -233,7 +236,7 @@ class FirewallPolicyInsertRule(command.Command):
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
         policy_id = client.find_firewall_policy(
-            parsed_args.firewall_policy)['id']
+            parsed_args.firewall_policy, ignore_missing=False)['id']
         body = self.args2body(parsed_args)
         client.insert_rule_into_policy(policy_id, **body)
         rule_id = body['firewall_rule_id']
@@ -261,7 +264,7 @@ class FirewallPolicyRemoveRule(command.Command):
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
         policy_id = client.find_firewall_policy(
-            parsed_args.firewall_policy)['id']
+            parsed_args.firewall_policy, ignore_missing=False)['id']
         fwr_id = _get_required_firewall_rule(client, parsed_args)
         body = {'firewall_rule_id': fwr_id}
         client.remove_rule_from_policy(policy_id, **body)
@@ -322,7 +325,7 @@ class SetFirewallPolicy(command.Command):
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
         fwp_id = client.find_firewall_policy(
-            parsed_args.firewall_policy)['id']
+            parsed_args.firewall_policy, ignore_missing=False)['id']
         attrs = _get_common_attrs(self.app.client_manager,
                                   parsed_args, is_create=False)
         try:
@@ -347,7 +350,7 @@ class ShowFirewallPolicy(command.ShowOne):
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
         fwp_id = client.find_firewall_policy(
-            parsed_args.firewall_policy)['id']
+            parsed_args.firewall_policy, ignore_missing=False)['id']
         obj = client.get_firewall_policy(fwp_id)
         display_columns, columns = utils.get_osc_show_columns_for_sdk_resource(
             obj, _attr_map_dict, ['location', 'tenant_id'])
@@ -359,7 +362,8 @@ def _get_required_firewall_rule(client, parsed_args):
     if not parsed_args.firewall_rule:
         msg = (_("Firewall rule (name or ID) is required."))
         raise exceptions.CommandError(msg)
-    return client.find_firewall_rule(parsed_args.firewall_rule)['id']
+    return client.find_firewall_rule(
+        parsed_args.firewall_rule, ignore_missing=False)['id']
 
 
 class UnsetFirewallPolicy(command.Command):
@@ -399,10 +403,11 @@ class UnsetFirewallPolicy(command.Command):
 
         if parsed_args.firewall_rule:
             current = client.find_firewall_policy(
-                parsed_args.firewall_policy)[const.FWRS]
+                parsed_args.firewall_policy, ignore_missing=False)[const.FWRS]
             removed = []
             for f in set(parsed_args.firewall_rule):
-                removed.append(client.find_firewall_rule(f)['id'])
+                removed.append(
+                    client.find_firewall_rule(f, ignore_missing=False)['id'])
             attrs[const.FWRS] = [r for r in current if r not in removed]
         if parsed_args.all_firewall_rule:
             attrs[const.FWRS] = []
@@ -415,7 +420,7 @@ class UnsetFirewallPolicy(command.Command):
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
         fwp_id = client.find_firewall_policy(
-            parsed_args.firewall_policy)['id']
+            parsed_args.firewall_policy, ignore_missing=False)['id']
         attrs = self._get_attrs(self.app.client_manager, parsed_args)
         try:
             client.update_firewall_policy(fwp_id, **attrs)

neutronclient/osc/v2/fwaas/firewallrule.py

@@ -229,12 +229,12 @@ def _get_common_attrs(client_manager, parsed_args, is_create=True):
         attrs['shared'] = False
     if parsed_args.source_firewall_group:
         attrs['source_firewall_group_id'] = client.find_firewall_group(
-            parsed_args.source_firewall_group)['id']
+            parsed_args.source_firewall_group, ignore_missing=False)['id']
     if parsed_args.no_source_firewall_group:
         attrs['source_firewall_group_id'] = None
     if parsed_args.destination_firewall_group:
         attrs['destination_firewall_group_id'] = client.find_firewall_group(
-            parsed_args.destination_firewall_group)['id']
+            parsed_args.destination_firewall_group, ignore_missing=False)['id']
     if parsed_args.no_destination_firewall_group:
         attrs['destination_firewall_group_id'] = None
     return attrs
@@ -284,7 +284,8 @@ class DeleteFirewallRule(command.Command):
         result = 0
         for fwr in parsed_args.firewall_rule:
             try:
-                fwr_id = client.find_firewall_rule(fwr)['id']
+                fwr_id = client.find_firewall_rule(
+                    fwr, ignore_missing=False)['id']
                 client.delete_firewall_rule(fwr_id)
             except Exception as e:
                 result += 1
@@ -361,7 +362,8 @@ class SetFirewallRule(command.Command):
         client = self.app.client_manager.network
         attrs = _get_common_attrs(self.app.client_manager,
                                   parsed_args, is_create=False)
-        fwr_id = client.find_firewall_rule(parsed_args.firewall_rule)['id']
+        fwr_id = client.find_firewall_rule(
+            parsed_args.firewall_rule, ignore_missing=False)['id']
         try:
             client.update_firewall_rule(fwr_id, **attrs)
         except Exception as e:
@@ -383,7 +385,8 @@ class ShowFirewallRule(command.ShowOne):
 
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
-        fwr_id = client.find_firewall_rule(parsed_args.firewall_rule)['id']
+        fwr_id = client.find_firewall_rule(
+            parsed_args.firewall_rule, ignore_missing=False)['id']
         obj = client.get_firewall_rule(fwr_id)
         display_columns, columns = utils.get_osc_show_columns_for_sdk_resource(
             obj, _attr_map_dict, ['location', 'tenant_id'])
@@ -461,7 +464,8 @@ class UnsetFirewallRule(command.Command):
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
         attrs = self._get_attrs(self.app.client_manager, parsed_args)
-        fwr_id = client.find_firewall_rule(parsed_args.firewall_rule)['id']
+        fwr_id = client.find_firewall_rule(
+            parsed_args.firewall_rule, ignore_missing=False)['id']
         try:
             client.update_firewall_rule(fwr_id, **attrs)
         except Exception as e:

neutronclient/tests/unit/osc/v2/fwaas/test_firewallgroup.py

@@ -215,7 +215,7 @@ class TestCreateFirewallGroup(TestFirewallGroup, common.TestCreateFWaaS):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         headers, data = self.cmd.take_action(parsed_args)
         self.networkclient.find_firewall_policy.assert_called_once_with(
-            ingress_policy)
+            ingress_policy, ignore_missing=False)
 
         self.check_results(headers, data, request)
 
@@ -236,7 +236,7 @@ class TestCreateFirewallGroup(TestFirewallGroup, common.TestCreateFWaaS):
         headers, data = self.cmd.take_action(parsed_args)
 
         self.networkclient.find_firewall_policy.assert_called_once_with(
-            egress_policy)
+            egress_policy, ignore_missing=False)
         self.check_results(headers, data, request)
 
     def test_create_with_all_params(self):
@@ -398,15 +398,15 @@ class TestSetFirewallGroup(TestFirewallGroup, common.TestSetFWaaS):
             # 1. Find specified firewall_group
             if self.networkclient.find_firewall_group.call_count == 1:
                 self.networkclient.find_firewall_group.assert_called_with(
-                    target)
+                    target, ignore_missing=False)
             # 2. Find specified 'ingress_firewall_policy'
             if self.networkclient.find_firewall_policy.call_count == 1:
                 self.networkclient.find_firewall_policy.assert_called_with(
-                    ingress_policy)
+                    ingress_policy, ignore_missing=False)
             # 3. Find specified 'ingress_firewall_policy'
             if self.networkclient.find_firewall_policy.call_count == 2:
                 self.networkclient.find_firewall_policy.assert_called_with(
-                    egress_policy)
+                    egress_policy, ignore_missing=False)
             return {'id': args[0]}
 
         self.networkclient.find_firewall_group.side_effect = _mock_fwg_policy
@@ -439,7 +439,7 @@ class TestSetFirewallGroup(TestFirewallGroup, common.TestSetFWaaS):
             # 1. Find specified firewall_group
             if self.networkclient.find_firewall_group.call_count in [1, 2]:
                 self.networkclient.find_firewall_group.assert_called_with(
-                    target)
+                    target, ignore_missing=False)
                 return {'id': args[0], 'ports': _fwg['ports']}
             # 2. Find specified 'port' #1
             if self.networkclient.find_port.call_count == 1:
@@ -687,7 +687,7 @@ class TestUnsetFirewallGroup(TestFirewallGroup, common.TestUnsetFWaaS):
             # 1. Find specified firewall_group
             if self.networkclient.find_firewall_group.call_count in [1, 2]:
                 self.networkclient.find_firewall_group.assert_called_with(
-                    target)
+                    target, ignore_missing=False)
                 return {'id': args[0], 'ports': _fwg['ports']}
             # 2. Find specified firewall_group and refer 'ports' attribute
             if self.networkclient.find_port.call_count == 2: