From e3341ea6256146133230ea1022fbc0d388cd8632 Mon Sep 17 00:00:00 2001 From: Bar RH Date: Mon, 13 Nov 2017 19:59:10 +0200 Subject: [PATCH] Extend loadbalancer_create valid VIP parameters combinations Octavia client was incoherent with Octavia's api-ref, hence it supported only a subset of the possible combinations for specifying VIP parameters in loadbalancer_create. Validation functions were updated and moved from utils.py to validate.py and load_balancer.py, accompanied by new unit tests. Aesthetics: Refined loadabalancer's and l7Policy's creation synopses. Task: 5825 Task: 5828 Story: 2001279 Change-Id: I1b4da3f32a9b58ed85f2abc462dee4d55b8afd47 --- octaviaclient/osc/v2/l7policy.py | 35 +++++++---- octaviaclient/osc/v2/l7rule.py | 5 +- octaviaclient/osc/v2/load_balancer.py | 27 ++++++-- octaviaclient/osc/v2/utils.py | 37 ----------- octaviaclient/osc/v2/validate.py | 38 +++++++++++ .../tests/unit/osc/v2/test_load_balancer.py | 29 +++++++++ .../tests/unit/osc/v2/test_validations.py | 63 +++++++++++++++++++ 7 files changed, 178 insertions(+), 56 deletions(-) create mode 100644 octaviaclient/osc/v2/validate.py create mode 100644 octaviaclient/tests/unit/osc/v2/test_validations.py diff --git a/octaviaclient/osc/v2/l7policy.py b/octaviaclient/osc/v2/l7policy.py index 8c2ff45..85dc721 100644 --- a/octaviaclient/osc/v2/l7policy.py +++ b/octaviaclient/osc/v2/l7policy.py @@ -20,6 +20,7 @@ from osc_lib import utils from octaviaclient.osc.v2 import constants as const from octaviaclient.osc.v2 import utils as v2_utils +from octaviaclient.osc.v2 import validate ACTION_CHOICES = ['REDIRECT_TO_URL', 'REDIRECT_TO_POOL', 'REJECT'] @@ -45,11 +46,7 @@ class CreateL7Policy(command.ShowOne): metavar='', help="Set l7policy description." ) - parser.add_argument( - '--redirect-pool', - metavar='', - help="Set the pool to redirect requests to (name or ID)." - ) + parser.add_argument( '--action', metavar="{REDIRECT_TO_URL,REDIRECT_TO_POOL,REJECT}", @@ -57,11 +54,19 @@ class CreateL7Policy(command.ShowOne): choices=ACTION_CHOICES, help="Set the action of the policy." ) - parser.add_argument( + + redirect_group = parser.add_mutually_exclusive_group() + redirect_group.add_argument( + '--redirect-pool', + metavar='', + help="Set the pool to redirect requests to (name or ID)." + ) + redirect_group.add_argument( '--redirect-url', metavar='', help="Set the URL to redirect requests to." ) + parser.add_argument( '--position', metavar='', @@ -88,7 +93,7 @@ class CreateL7Policy(command.ShowOne): rows = const.L7POLICY_ROWS attrs = v2_utils.get_l7policy_attrs(self.app.client_manager, parsed_args) - v2_utils.check_l7policy_attrs(attrs) + validate.check_l7policy_attrs(attrs) body = {"l7policy": attrs} data = self.app.client_manager.load_balancer.l7policy_create( @@ -195,22 +200,25 @@ class SetL7Policy(command.Command): metavar='', help="Set l7policy description." ) - parser.add_argument( - '--redirect-pool', - metavar='', - help="Set the pool to redirect requests to (name or ID)." - ) parser.add_argument( '--action', metavar="{REDIRECT_TO_URL,REDIRECT_TO_POOL,REJECT}", choices=ACTION_CHOICES, help="Set the action of the policy." ) - parser.add_argument( + + redirect_group = parser.add_mutually_exclusive_group() + redirect_group.add_argument( + '--redirect-pool', + metavar='', + help="Set the pool to redirect requests to (name or ID)." + ) + redirect_group.add_argument( '--redirect-url', metavar='', help="Set the URL to redirect requests to." ) + parser.add_argument( '--position', metavar='', @@ -237,6 +245,7 @@ class SetL7Policy(command.Command): attrs = v2_utils.get_l7policy_attrs(self.app.client_manager, parsed_args) + validate.check_l7policy_attrs(attrs) l7policy_id = attrs.pop('l7policy_id') body = {'l7policy': attrs} diff --git a/octaviaclient/osc/v2/l7rule.py b/octaviaclient/osc/v2/l7rule.py index 4850603..7811b6a 100644 --- a/octaviaclient/osc/v2/l7rule.py +++ b/octaviaclient/osc/v2/l7rule.py @@ -20,6 +20,7 @@ from osc_lib import utils from octaviaclient.osc.v2 import constants as const from octaviaclient.osc.v2 import utils as v2_utils +from octaviaclient.osc.v2 import validate COMPARE_TYPES = ['REGEX', 'EQUAL_TO', 'CONTAINS', 'ENDS_WITH', 'STARTS_WITH'] TYPES = ['FILE_TYPE', 'PATH', 'COOKIE', 'HOST_NAME', 'HEADER'] @@ -86,7 +87,7 @@ class CreateL7Rule(command.ShowOne): rows = const.L7RULE_ROWS attrs = v2_utils.get_l7rule_attrs(self.app.client_manager, parsed_args) - v2_utils.check_l7rule_attrs(attrs) + validate.check_l7rule_attrs(attrs) l7policy_id = attrs.pop('l7policy_id') body = {"rule": attrs} @@ -248,7 +249,7 @@ class SetL7Rule(command.Command): def take_action(self, parsed_args): attrs = v2_utils.get_l7rule_attrs(self.app.client_manager, parsed_args) - v2_utils.check_l7rule_attrs(attrs) + validate.check_l7rule_attrs(attrs) l7policy_id = attrs.pop('l7policy_id') l7rule_id = attrs.pop('l7rule_id') diff --git a/octaviaclient/osc/v2/load_balancer.py b/octaviaclient/osc/v2/load_balancer.py index 060b021..404b0d3 100644 --- a/octaviaclient/osc/v2/load_balancer.py +++ b/octaviaclient/osc/v2/load_balancer.py @@ -15,6 +15,7 @@ from cliff import lister from osc_lib.command import command +from osc_lib import exceptions from osc_lib import utils from octaviaclient.osc.v2 import constants as const @@ -24,6 +25,18 @@ from octaviaclient.osc.v2 import utils as v2_utils class CreateLoadBalancer(command.ShowOne): """Create a load balancer""" + @staticmethod + def _check_attrs(attrs): + verify_args = ['vip_subnet_id', 'vip_network_id', 'vip_port_id'] + if not any(i in attrs for i in verify_args): + msg = ("Missing required argument: Requires one of " + "--vip-subnet-id, --vip-network-id or --vip-port-id") + raise exceptions.CommandError(msg) + if all(i in attrs for i in ('vip_network_id', 'vip_port_id')): + msg = ("Argument error: --vip-port-id can not be used with " + "--vip-network-id") + raise exceptions.CommandError(msg) + def get_parser(self, prog_name): parser = super(CreateLoadBalancer, self).get_parser(prog_name) @@ -42,21 +55,27 @@ class CreateLoadBalancer(command.ShowOne): metavar='', help="Set the VIP IP Address." ) - parser.add_argument( + + vip_group = parser.add_argument_group( + "VIP Network", + description="At least one of the following arguments is required." + ) + vip_group.add_argument( '--vip-port-id', metavar='', help="Set Port for the load balancer (name or ID)." ) - parser.add_argument( + vip_group.add_argument( '--vip-subnet-id', metavar='', help="Set subnet for the load balancer (name or ID)." ) - parser.add_argument( + vip_group.add_argument( '--vip-network-id', metavar='', help="Set network for the load balancer (name or ID)." ) + parser.add_argument( '--project', metavar='', @@ -82,7 +101,7 @@ class CreateLoadBalancer(command.ShowOne): rows = const.LOAD_BALANCER_ROWS attrs = v2_utils.get_loadbalancer_attrs(self.app.client_manager, parsed_args) - v2_utils.check_loadbalancer_attrs(attrs) + self._check_attrs(attrs) body = {'loadbalancer': attrs} data = self.app.client_manager.load_balancer.load_balancer_create( diff --git a/octaviaclient/osc/v2/utils.py b/octaviaclient/osc/v2/utils.py index 249908e..94b6572 100644 --- a/octaviaclient/osc/v2/utils.py +++ b/octaviaclient/osc/v2/utils.py @@ -116,19 +116,6 @@ def get_loadbalancer_attrs(client_manager, parsed_args): return attrs -def check_loadbalancer_attrs(attrs): - verify_args = ['vip_subnet_id', 'vip_network_id', 'vip_port_id'] - if not any(i in attrs.keys() for i in verify_args): - msg = ("Missing required argument: Requires one of --vip-subnet-id, " - "--vip-network-id or --vip-port-id") - raise exceptions.CommandError(msg) - elif 'vip_port_id' in attrs: - if any(i in attrs.keys() for i in ['vip_subnet_id', 'vip_address']): - msg = ("Argument error: --port-id can not be used with " - "--vip-network-id or --vip-subnet-id") - raise exceptions.CommandError(msg) - - def get_listener_attrs(client_manager, parsed_args): attr_map = { 'name': ('name', str), @@ -282,18 +269,6 @@ def get_l7policy_attrs(client_manager, parsed_args): return attrs -def check_l7policy_attrs(attrs): - msg = None - if attrs['action'] == 'REDIRECT_TO_POOL': - if 'redirect_pool_id' not in attrs: - msg = 'Missing argument: --redirect-pool' - elif attrs['action'] == 'REDIRECT_TO_URL': - if 'redirect_url' not in attrs: - msg = 'Missing argument: --redirect-url' - if msg is not None: - raise exceptions.CommandError(msg) - - def get_l7rule_attrs(client_manager, parsed_args): attr_map = { 'action': ('action', str), @@ -328,18 +303,6 @@ def get_l7rule_attrs(client_manager, parsed_args): return attrs -def check_l7rule_attrs(attrs): - msg = None - if 'type' in attrs.keys() and attrs['type'] == 'COOKIE': - if 'key' not in attrs: - msg = 'Missing argument: --type COOKIE requires --key ' - elif 'type' in attrs.keys() and attrs['type'] == 'HEADER': - if 'key' not in attrs: - msg = 'Missing argument: --type HEADER requires --key ' - if msg is not None: - raise exceptions.CommandError(msg) - - def get_health_monitor_attrs(client_manager, parsed_args): attr_map = { 'health_monitor': ( diff --git a/octaviaclient/osc/v2/validate.py b/octaviaclient/osc/v2/validate.py new file mode 100644 index 0000000..10ac08a --- /dev/null +++ b/octaviaclient/osc/v2/validate.py @@ -0,0 +1,38 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +from osc_lib import exceptions + + +def check_l7policy_attrs(attrs): + msg = None + if 'action' not in attrs: + return + if attrs['action'] == 'REDIRECT_TO_POOL': + if 'redirect_pool_id' not in attrs: + msg = 'Missing argument: --redirect-pool' + elif attrs['action'] == 'REDIRECT_TO_URL': + if 'redirect_url' not in attrs: + msg = 'Missing argument: --redirect-url' + if msg is not None: + raise exceptions.CommandError(msg) + + +def check_l7rule_attrs(attrs): + if 'type' in attrs: + if attrs['type'] in ('COOKIE', 'HEADER'): + if 'key' not in attrs: + msg = ( + "Missing argument: --type {type_name} requires " + "--key ".format(type_name=attrs['type'])) + raise exceptions.CommandError(msg) diff --git a/octaviaclient/tests/unit/osc/v2/test_load_balancer.py b/octaviaclient/tests/unit/osc/v2/test_load_balancer.py index f8a1878..e9b9e09 100644 --- a/octaviaclient/tests/unit/osc/v2/test_load_balancer.py +++ b/octaviaclient/tests/unit/osc/v2/test_load_balancer.py @@ -12,6 +12,7 @@ # import copy +import itertools import mock from osc_lib import exceptions @@ -159,6 +160,34 @@ class TestLoadBalancerCreate(TestLoadBalancer): self.api_mock.load_balancer_create.assert_called_with( json={'loadbalancer': self.lb_info['loadbalancers'][0]}) + @mock.patch('octaviaclient.osc.v2.utils.get_loadbalancer_attrs') + def test_load_balancer_create_missing_args(self, mock_client): + attrs_list = self.lb_info['loadbalancers'][0] + args = ("vip_subnet_id", "vip_network_id", "vip_port_id") + for a in args: + # init missing keys + attrs_list[a] = '' + # verify all valid combinations of args + for n in range(len(args)+1): + for comb in itertools.combinations(args, n): + # subtract comb's keys from attrs_list + filtered_attrs = {k: v for k, v in attrs_list.items() if ( + k not in comb)} + mock_client.return_value = filtered_attrs + if not any(k in filtered_attrs for k in args) or all( + k in filtered_attrs for k in ("vip_network_id", + "vip_port_id") + ): + self.assertRaises( + exceptions.CommandError, + self.cmd.take_action, + filtered_attrs) + else: + try: + self.cmd.take_action(filtered_attrs) + except exceptions.CommandError as e: + self.fail("%s raised unexpectedly" % e) + class TestLoadBalancerShow(TestLoadBalancer): diff --git a/octaviaclient/tests/unit/osc/v2/test_validations.py b/octaviaclient/tests/unit/osc/v2/test_validations.py new file mode 100644 index 0000000..3992d06 --- /dev/null +++ b/octaviaclient/tests/unit/osc/v2/test_validations.py @@ -0,0 +1,63 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +from octaviaclient.osc.v2 import validate +from osc_lib import exceptions +from osc_lib.tests import utils + + +class TestValidations(utils.TestCommand): + def setUp(self): + super(TestValidations, self).setUp() + + def test_check_l7policy_attrs(self): + attrs_dict = { + "action": "redirect_to_pool".upper(), + "redirect_pool_id": "id", + } + try: + validate.check_l7policy_attrs(attrs_dict) + except exceptions.CommandError as e: + self.fail("%s raised unexpectedly" % e) + attrs_dict.pop("redirect_pool_id") + self.assertRaises( + exceptions.CommandError, + validate.check_l7policy_attrs, attrs_dict) + + attrs_dict = { + "action": "redirect_to_url".upper(), + "redirect_url": "url", + } + try: + validate.check_l7policy_attrs(attrs_dict) + except exceptions.CommandError as e: + self.fail("%s raised unexpectedly" % e) + attrs_dict.pop("redirect_url") + self.assertRaises( + exceptions.CommandError, + validate.check_l7policy_attrs, attrs_dict) + + def test_check_l7rule_attrs(self): + for i in ("cookie", "header"): + attrs_dict = { + "type": i.upper(), + "key": "key", + } + try: + validate.check_l7rule_attrs(attrs_dict) + except exceptions.CommandError as e: + self.fail("%s raised unexpectedly" % e) + attrs_dict.pop("key") + self.assertRaises( + exceptions.CommandError, + validate.check_l7rule_attrs, attrs_dict)