From 3bbcdf8ff21724eac6e4668143eb709f05808d85 Mon Sep 17 00:00:00 2001 From: ZhaoBo Date: Wed, 8 Mar 2017 15:14:59 +0800 Subject: [PATCH] Don't sort the fw_rule order in OSC Currently, the FWaaS accept the "fw_rule_list" argrument in fw_policy update, and it will update all the related fw_rules association. If the order is different, the behavior is not expected, so we should take care of the input value order, and maintain the original one. This patch remove the "sorted" function when construct the firewall policy request body, and follow the input order. Closes-Bug: #1671338 Change-Id: Iaa45446eee199582f5f11a4705e82fb0ab644bcc --- neutronclient/osc/v2/fwaas/firewallpolicy.py | 20 +++++++++---------- .../unit/osc/v2/fwaas/test_firewallpolicy.py | 12 +++++------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/neutronclient/osc/v2/fwaas/firewallpolicy.py b/neutronclient/osc/v2/fwaas/firewallpolicy.py index 5ac0971..288bac0 100644 --- a/neutronclient/osc/v2/fwaas/firewallpolicy.py +++ b/neutronclient/osc/v2/fwaas/firewallpolicy.py @@ -54,20 +54,20 @@ def _get_common_attrs(client_manager, parsed_args, is_create=True): ).id if parsed_args.firewall_rule and parsed_args.no_firewall_rule: _firewall_rules = [] - for f in set(parsed_args.firewall_rule): + for f in parsed_args.firewall_rule: _firewall_rules.append(client.find_resource( const.FWR, f, cmd_resource=const.CMD_FWR)['id']) - attrs[const.FWRS] = sorted(_firewall_rules) + attrs[const.FWRS] = _firewall_rules elif parsed_args.firewall_rule: rules = [] - for f in set(parsed_args.firewall_rule): - rules.append(client.find_resource( - const.FWR, f, cmd_resource=const.CMD_FWR)['id']) if not is_create: rules += client.find_resource( const.FWP, parsed_args.firewall_policy, cmd_resource=const.CMD_FWP)[const.FWRS] - attrs[const.FWRS] = sorted(set(rules)) + for f in parsed_args.firewall_rule: + rules.append(client.find_resource( + const.FWR, f, cmd_resource=const.CMD_FWR)['id']) + attrs[const.FWRS] = rules elif parsed_args.no_firewall_rule: attrs[const.FWRS] = [] if parsed_args.audited: @@ -395,14 +395,14 @@ class UnsetFirewallPolicy(command.Command): client = client_manager.neutronclient if parsed_args.firewall_rule: - old = client.find_resource( + current = client.find_resource( const.FWP, parsed_args.firewall_policy, cmd_resource=const.CMD_FWP)[const.FWRS] - new = [] + removed = [] for f in set(parsed_args.firewall_rule): - new.append(client.find_resource( + removed.append(client.find_resource( const.FWR, f, cmd_resource=const.CMD_FWR)['id']) - attrs[const.FWRS] = sorted(list(set(old) - set(new))) + attrs[const.FWRS] = [r for r in current if r not in removed] if parsed_args.all_firewall_rule: attrs[const.FWRS] = [] if parsed_args.audited: diff --git a/neutronclient/tests/unit/osc/v2/fwaas/test_firewallpolicy.py b/neutronclient/tests/unit/osc/v2/fwaas/test_firewallpolicy.py index 49e4321..6d97807 100644 --- a/neutronclient/tests/unit/osc/v2/fwaas/test_firewallpolicy.py +++ b/neutronclient/tests/unit/osc/v2/fwaas/test_firewallpolicy.py @@ -343,19 +343,19 @@ class TestSetFirewallPolicy(TestFirewallPolicy, common.TestSetFWaaS): if self.neutronclient.find_resource.call_count == 1: self.neutronclient.find_resource.assert_called_with( self.res, target, cmd_resource=const.CMD_FWP) - # 2. Find specified firewall_rule + # 2. Find specified firewall_policy's 'firewall_rules' attribute if self.neutronclient.find_resource.call_count == 2: self.neutronclient.find_resource.assert_called_with( - 'firewall_rule', args[1], cmd_resource=const.CMD_FWR) + self.res, args[1], cmd_resource=const.CMD_FWP) + return {'firewall_rules': _fwp['firewall_rules']} # 3. Find specified firewall_rule if self.neutronclient.find_resource.call_count == 3: self.neutronclient.find_resource.assert_called_with( 'firewall_rule', args[1], cmd_resource=const.CMD_FWR) - # 4. Find specified firewall_policy's 'firewall_rules' attribute + # 4. Find specified firewall_rule if self.neutronclient.find_resource.call_count == 4: self.neutronclient.find_resource.assert_called_with( - self.res, target, cmd_resource=const.CMD_FWP) - return {'firewall_rules': _fwp['firewall_rules']} + 'firewall_rule', args[1], cmd_resource=const.CMD_FWR) return {'id': args[1]} self.neutronclient.find_resource.side_effect = _mock_policy @@ -372,7 +372,7 @@ class TestSetFirewallPolicy(TestFirewallPolicy, common.TestSetFWaaS): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - expect = sorted(set(_fwp['firewall_rules'] + [rule1, rule2])) + expect = _fwp['firewall_rules'] + [rule1, rule2] body = {self.res: {'firewall_rules': expect}} self.mocked.assert_called_once_with(target, body) self.assertEqual(4, self.neutronclient.find_resource.call_count)