Skip to content

Commit 6e5eb4d

Browse files
slawqoslawqo
committed
[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>
1 parent f7c0850 commit 6e5eb4d

File tree

4 files changed

+50
-37
lines changed

4 files changed

+50
-37
lines changed

‎neutronclient/osc/v2/fwaas/firewallgroup.py‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -129,19 +129,19 @@ def _get_common_attrs(client_manager, parsed_args, is_create=True):
129129
if (parsed_args.ingress_firewall_policy and
130130
parsed_args.no_ingress_firewall_policy):
131131
attrs['ingress_firewall_policy_id'] = client.find_firewall_policy(
132-
parsed_args.ingress_firewall_policy)['id']
132+
parsed_args.ingress_firewall_policy, ignore_missing=False)['id']
133133
elif parsed_args.ingress_firewall_policy:
134134
attrs['ingress_firewall_policy_id'] = client.find_firewall_policy(
135-
parsed_args.ingress_firewall_policy)['id']
135+
parsed_args.ingress_firewall_policy, ignore_missing=False)['id']
136136
elif parsed_args.no_ingress_firewall_policy:
137137
attrs['ingress_firewall_policy_id'] = None
138138
if (parsed_args.egress_firewall_policy and
139139
parsed_args.no_egress_firewall_policy):
140140
attrs['egress_firewall_policy_id'] = client.find_firewall_policy(
141-
parsed_args.egress_firewall_policy)['id']
141+
parsed_args.egress_firewall_policy, ignore_missing=False)['id']
142142
elif parsed_args.egress_firewall_policy:
143143
attrs['egress_firewall_policy_id'] = client.find_firewall_policy(
144-
parsed_args.egress_firewall_policy)['id']
144+
parsed_args.egress_firewall_policy, ignore_missing=False)['id']
145145
elif parsed_args.no_egress_firewall_policy:
146146
attrs['egress_firewall_policy_id'] = None
147147
if parsed_args.share:
@@ -165,7 +165,7 @@ def _get_common_attrs(client_manager, parsed_args, is_create=True):
165165
ports.append(client.find_port(p)['id'])
166166
if not is_create:
167167
ports += client.find_firewall_group(
168-
parsed_args.firewall_group)['ports']
168+
parsed_args.firewall_group, ignore_missing=False)['ports']
169169
attrs['ports'] = sorted(set(ports))
170170
elif parsed_args.no_port:
171171
attrs['ports'] = []
@@ -220,7 +220,8 @@ def take_action(self, parsed_args):
220220
result = 0
221221
for fwg in parsed_args.firewall_group:
222222
try:
223-
fwg_id = client.find_firewall_group(fwg)['id']
223+
fwg_id = client.find_firewall_group(
224+
fwg, ignore_missing=False)['id']
224225
client.delete_firewall_group(fwg_id)
225226
except Exception as e:
226227
result += 1
@@ -281,7 +282,8 @@ def get_parser(self, prog_name):
281282

282283
def take_action(self, parsed_args):
283284
client = self.app.client_manager.network
284-
fwg_id = client.find_firewall_group(parsed_args.firewall_group)['id']
285+
fwg_id = client.find_firewall_group(
286+
parsed_args.firewall_group, ignore_missing=False)['id']
285287
attrs = _get_common_attrs(self.app.client_manager, parsed_args,
286288
is_create=False)
287289
try:
@@ -305,7 +307,8 @@ def get_parser(self, prog_name):
305307

306308
def take_action(self, parsed_args):
307309
client = self.app.client_manager.network
308-
fwg_id = client.find_firewall_group(parsed_args.firewall_group)['id']
310+
fwg_id = client.find_firewall_group(
311+
parsed_args.firewall_group, ignore_missing=False)['id']
309312
obj = client.get_firewall_group(fwg_id)
310313
display_columns, columns = utils.get_osc_show_columns_for_sdk_resource(
311314
obj, _attr_map_dict, ['location', 'tenant_id'])
@@ -366,7 +369,7 @@ def _get_attrs(self, client, parsed_args):
366369
attrs['admin_state_up'] = False
367370
if parsed_args.port:
368371
old = client.find_firewall_group(
369-
parsed_args.firewall_group)['ports']
372+
parsed_args.firewall_group, ignore_missing=False)['ports']
370373
new = [client.find_port(r)['id'] for r in parsed_args.port]
371374
attrs['ports'] = sorted(list(set(old) - set(new)))
372375
if parsed_args.all_port:
@@ -375,7 +378,8 @@ def _get_attrs(self, client, parsed_args):
375378

376379
def take_action(self, parsed_args):
377380
client = self.app.client_manager.network
378-
fwg_id = client.find_firewall_group(parsed_args.firewall_group)['id']
381+
fwg_id = client.find_firewall_group(
382+
parsed_args.firewall_group, ignore_missing=False)['id']
379383
attrs = self._get_attrs(client, parsed_args)
380384
try:
381385
client.update_firewall_group(fwg_id, **attrs)

‎neutronclient/osc/v2/fwaas/firewallpolicy.py‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,18 @@ def _get_common_attrs(client_manager, parsed_args, is_create=True):
6767
if parsed_args.firewall_rule and parsed_args.no_firewall_rule:
6868
_firewall_rules = []
6969
for f in parsed_args.firewall_rule:
70-
_firewall_rules.append(client.find_firewall_rule(f)['id'])
70+
_firewall_rules.append(
71+
client.find_firewall_rule(f, ignore_missing=False)['id'])
7172
attrs[const.FWRS] = _firewall_rules
7273
elif parsed_args.firewall_rule:
7374
rules = []
7475
if not is_create:
7576
foobar = client.find_firewall_policy(
76-
parsed_args.firewall_policy)
77+
parsed_args.firewall_policy, ignore_missing=False)
7778
rules += foobar[const.FWRS]
7879
for f in parsed_args.firewall_rule:
79-
rules.append(client.find_firewall_rule(f)['id'])
80+
rules.append(
81+
client.find_firewall_rule(f, ignore_missing=False)['id'])
8082
attrs[const.FWRS] = rules
8183
elif parsed_args.no_firewall_rule:
8284
attrs[const.FWRS] = []
@@ -173,7 +175,8 @@ def take_action(self, parsed_args):
173175
result = 0
174176
for fwp in parsed_args.firewall_policy:
175177
try:
176-
fwp_id = client.find_firewall_policy(fwp)['id']
178+
fwp_id = client.find_firewall_policy(
179+
fwp, ignore_missing=False)['id']
177180
client.delete_firewall_policy(fwp_id)
178181
except Exception as e:
179182
result += 1
@@ -220,20 +223,20 @@ def args2body(self, parsed_args):
220223
if 'insert_before' in parsed_args:
221224
if parsed_args.insert_before:
222225
_insert_before = client.find_firewall_rule(
223-
parsed_args.insert_before)['id']
226+
parsed_args.insert_before, ignore_missing=False)['id']
224227
_insert_after = ''
225228
if 'insert_after' in parsed_args:
226229
if parsed_args.insert_after:
227230
_insert_after = client.find_firewall_rule(
228-
parsed_args.insert_after)['id']
231+
parsed_args.insert_after, ignore_missing=False)['id']
229232
return {'firewall_rule_id': _rule_id,
230233
'insert_before': _insert_before,
231234
'insert_after': _insert_after}
232235

233236
def take_action(self, parsed_args):
234237
client = self.app.client_manager.network
235238
policy_id = client.find_firewall_policy(
236-
parsed_args.firewall_policy)['id']
239+
parsed_args.firewall_policy, ignore_missing=False)['id']
237240
body = self.args2body(parsed_args)
238241
client.insert_rule_into_policy(policy_id, **body)
239242
rule_id = body['firewall_rule_id']
@@ -261,7 +264,7 @@ def get_parser(self, prog_name):
261264
def take_action(self, parsed_args):
262265
client = self.app.client_manager.network
263266
policy_id = client.find_firewall_policy(
264-
parsed_args.firewall_policy)['id']
267+
parsed_args.firewall_policy, ignore_missing=False)['id']
265268
fwr_id = _get_required_firewall_rule(client, parsed_args)
266269
body = {'firewall_rule_id': fwr_id}
267270
client.remove_rule_from_policy(policy_id, **body)
@@ -322,7 +325,7 @@ def get_parser(self, prog_name):
322325
def take_action(self, parsed_args):
323326
client = self.app.client_manager.network
324327
fwp_id = client.find_firewall_policy(
325-
parsed_args.firewall_policy)['id']
328+
parsed_args.firewall_policy, ignore_missing=False)['id']
326329
attrs = _get_common_attrs(self.app.client_manager,
327330
parsed_args, is_create=False)
328331
try:
@@ -347,7 +350,7 @@ def get_parser(self, prog_name):
347350
def take_action(self, parsed_args):
348351
client = self.app.client_manager.network
349352
fwp_id = client.find_firewall_policy(
350-
parsed_args.firewall_policy)['id']
353+
parsed_args.firewall_policy, ignore_missing=False)['id']
351354
obj = client.get_firewall_policy(fwp_id)
352355
display_columns, columns = utils.get_osc_show_columns_for_sdk_resource(
353356
obj, _attr_map_dict, ['location', 'tenant_id'])
@@ -359,7 +362,8 @@ def _get_required_firewall_rule(client, parsed_args):
359362
if not parsed_args.firewall_rule:
360363
msg = (_("Firewall rule (name or ID) is required."))
361364
raise exceptions.CommandError(msg)
362-
return client.find_firewall_rule(parsed_args.firewall_rule)['id']
365+
return client.find_firewall_rule(
366+
parsed_args.firewall_rule, ignore_missing=False)['id']
363367

364368

365369
class UnsetFirewallPolicy(command.Command):
@@ -399,10 +403,11 @@ def _get_attrs(self, client_manager, parsed_args):
399403

400404
if parsed_args.firewall_rule:
401405
current = client.find_firewall_policy(
402-
parsed_args.firewall_policy)[const.FWRS]
406+
parsed_args.firewall_policy, ignore_missing=False)[const.FWRS]
403407
removed = []
404408
for f in set(parsed_args.firewall_rule):
405-
removed.append(client.find_firewall_rule(f)['id'])
409+
removed.append(
410+
client.find_firewall_rule(f, ignore_missing=False)['id'])
406411
attrs[const.FWRS] = [r for r in current if r not in removed]
407412
if parsed_args.all_firewall_rule:
408413
attrs[const.FWRS] = []
@@ -415,7 +420,7 @@ def _get_attrs(self, client_manager, parsed_args):
415420
def take_action(self, parsed_args):
416421
client = self.app.client_manager.network
417422
fwp_id = client.find_firewall_policy(
418-
parsed_args.firewall_policy)['id']
423+
parsed_args.firewall_policy, ignore_missing=False)['id']
419424
attrs = self._get_attrs(self.app.client_manager, parsed_args)
420425
try:
421426
client.update_firewall_policy(fwp_id, **attrs)

‎neutronclient/osc/v2/fwaas/firewallrule.py‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -229,12 +229,12 @@ def _get_common_attrs(client_manager, parsed_args, is_create=True):
229229
attrs['shared'] = False
230230
if parsed_args.source_firewall_group:
231231
attrs['source_firewall_group_id'] = client.find_firewall_group(
232-
parsed_args.source_firewall_group)['id']
232+
parsed_args.source_firewall_group, ignore_missing=False)['id']
233233
if parsed_args.no_source_firewall_group:
234234
attrs['source_firewall_group_id'] = None
235235
if parsed_args.destination_firewall_group:
236236
attrs['destination_firewall_group_id'] = client.find_firewall_group(
237-
parsed_args.destination_firewall_group)['id']
237+
parsed_args.destination_firewall_group, ignore_missing=False)['id']
238238
if parsed_args.no_destination_firewall_group:
239239
attrs['destination_firewall_group_id'] = None
240240
return attrs
@@ -284,7 +284,8 @@ def take_action(self, parsed_args):
284284
result = 0
285285
for fwr in parsed_args.firewall_rule:
286286
try:
287-
fwr_id = client.find_firewall_rule(fwr)['id']
287+
fwr_id = client.find_firewall_rule(
288+
fwr, ignore_missing=False)['id']
288289
client.delete_firewall_rule(fwr_id)
289290
except Exception as e:
290291
result += 1
@@ -361,7 +362,8 @@ def take_action(self, parsed_args):
361362
client = self.app.client_manager.network
362363
attrs = _get_common_attrs(self.app.client_manager,
363364
parsed_args, is_create=False)
364-
fwr_id = client.find_firewall_rule(parsed_args.firewall_rule)['id']
365+
fwr_id = client.find_firewall_rule(
366+
parsed_args.firewall_rule, ignore_missing=False)['id']
365367
try:
366368
client.update_firewall_rule(fwr_id, **attrs)
367369
except Exception as e:
@@ -383,7 +385,8 @@ def get_parser(self, prog_name):
383385

384386
def take_action(self, parsed_args):
385387
client = self.app.client_manager.network
386-
fwr_id = client.find_firewall_rule(parsed_args.firewall_rule)['id']
388+
fwr_id = client.find_firewall_rule(
389+
parsed_args.firewall_rule, ignore_missing=False)['id']
387390
obj = client.get_firewall_rule(fwr_id)
388391
display_columns, columns = utils.get_osc_show_columns_for_sdk_resource(
389392
obj, _attr_map_dict, ['location', 'tenant_id'])
@@ -461,7 +464,8 @@ def _get_attrs(self, client_manager, parsed_args):
461464
def take_action(self, parsed_args):
462465
client = self.app.client_manager.network
463466
attrs = self._get_attrs(self.app.client_manager, parsed_args)
464-
fwr_id = client.find_firewall_rule(parsed_args.firewall_rule)['id']
467+
fwr_id = client.find_firewall_rule(
468+
parsed_args.firewall_rule, ignore_missing=False)['id']
465469
try:
466470
client.update_firewall_rule(fwr_id, **attrs)
467471
except Exception as e:

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ def _mock_port_fwg(*args, **kwargs):
215215
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
216216
headers, data = self.cmd.take_action(parsed_args)
217217
self.networkclient.find_firewall_policy.assert_called_once_with(
218-
ingress_policy)
218+
ingress_policy, ignore_missing=False)
219219

220220
self.check_results(headers, data, request)
221221

@@ -236,7 +236,7 @@ def _mock_find(*args, **kwargs):
236236
headers, data = self.cmd.take_action(parsed_args)
237237

238238
self.networkclient.find_firewall_policy.assert_called_once_with(
239-
egress_policy)
239+
egress_policy, ignore_missing=False)
240240
self.check_results(headers, data, request)
241241

242242
def test_create_with_all_params(self):
@@ -398,15 +398,15 @@ def _mock_fwg_policy(*args, **kwargs):
398398
# 1. Find specified firewall_group
399399
if self.networkclient.find_firewall_group.call_count == 1:
400400
self.networkclient.find_firewall_group.assert_called_with(
401-
target)
401+
target, ignore_missing=False)
402402
# 2. Find specified 'ingress_firewall_policy'
403403
if self.networkclient.find_firewall_policy.call_count == 1:
404404
self.networkclient.find_firewall_policy.assert_called_with(
405-
ingress_policy)
405+
ingress_policy, ignore_missing=False)
406406
# 3. Find specified 'ingress_firewall_policy'
407407
if self.networkclient.find_firewall_policy.call_count == 2:
408408
self.networkclient.find_firewall_policy.assert_called_with(
409-
egress_policy)
409+
egress_policy, ignore_missing=False)
410410
return {'id': args[0]}
411411

412412
self.networkclient.find_firewall_group.side_effect = _mock_fwg_policy
@@ -439,7 +439,7 @@ def _mock_port_fwg(*args, **kwargs):
439439
# 1. Find specified firewall_group
440440
if self.networkclient.find_firewall_group.call_count in [1, 2]:
441441
self.networkclient.find_firewall_group.assert_called_with(
442-
target)
442+
target, ignore_missing=False)
443443
return {'id': args[0], 'ports': _fwg['ports']}
444444
# 2. Find specified 'port' #1
445445
if self.networkclient.find_port.call_count == 1:
@@ -687,7 +687,7 @@ def _mock_port_fwg(*args, **kwargs):
687687
# 1. Find specified firewall_group
688688
if self.networkclient.find_firewall_group.call_count in [1, 2]:
689689
self.networkclient.find_firewall_group.assert_called_with(
690-
target)
690+
target, ignore_missing=False)
691691
return {'id': args[0], 'ports': _fwg['ports']}
692692
# 2. Find specified firewall_group and refer 'ports' attribute
693693
if self.networkclient.find_port.call_count == 2:

0 commit comments

Comments
 (0)