From ee793c65d67ee22286291c13d2f4c3269e5c8409 Mon Sep 17 00:00:00 2001 From: Gregory Thiemonge Date: Fri, 5 Apr 2024 11:18:15 -0400 Subject: [PATCH] Add vip_sg_ids to the API Change-Id: I87ee5567eb6a2432ae69dd6f24c61e72e44850e6 --- api-ref/source/parameters.yaml | 16 +++ api-ref/source/v2/loadbalancer.inc | 12 +++ doc/source/contributor/guides/providers.rst | 6 ++ .../feature-matrix-lb.ini | 8 ++ octavia/api/root_controller.py | 5 +- octavia/api/v2/controllers/listener.py | 14 ++- octavia/api/v2/controllers/load_balancer.py | 27 ++++- octavia/api/v2/types/load_balancer.py | 8 +- octavia/common/utils.py | 5 +- octavia/common/validate.py | 16 +++ .../functional/api/v2/test_load_balancer.py | 99 ++++++++++++++++++- .../add_vip_sg_ids-feaeaf8b3301e267.yaml | 6 ++ 12 files changed, 215 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/add_vip_sg_ids-feaeaf8b3301e267.yaml diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index e752979a83..929043d722 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -1780,6 +1780,22 @@ vip_qos_policy_id-optional: in: body required: false type: uuid +vip_sg_ids: + description: | + The list of Security Group IDs of the Virtual IP (VIP) port of the Load + Balancer. + in: body + required: true + type: array + min_version: 2.29 +vip_sg_ids-optional: + description: | + The list of Security Group IDs of the Virtual IP (VIP) port of the Load + Balancer. + in: body + required: false + type: array + min_version: 2.29 vip_subnet_id: description: | The ID of the subnet for the Virtual IP (VIP). diff --git a/api-ref/source/v2/loadbalancer.inc b/api-ref/source/v2/loadbalancer.inc index f67794c2ba..feb5627211 100644 --- a/api-ref/source/v2/loadbalancer.inc +++ b/api-ref/source/v2/loadbalancer.inc @@ -67,6 +67,7 @@ Response Parameters - vip_port_id: vip_port_id - vip_qos_policy_id: vip_qos_policy_id - vip_subnet_id: vip_subnet_id + - vip_sg_ids: vip_sg_ids - vip_vnic_type: vip_vnic_type Response Example @@ -152,6 +153,13 @@ providing a list of JSON objects containing a ``subnet_id`` and optionally an ``ip_address``. All additional subnets must be part of the same network as the primary VIP. +An optional ``vip_sg_ids`` attribute can be used to set custom Neutron Security +Groups that are applied on the VIP port of the Load Balancer. When this option +is used, Octavia does not manage the security of the Listeners, the user +must set Security Group Rules to allow the network traffic on the VIP port. +``vip_sg_ids`` are incompatible with SR-IOV load balancer and cannot be set if +the load balancer has a listener that uses ``allowed_cidrs``. + .. rest_status_code:: success ../http-status.yaml - 201 @@ -186,6 +194,7 @@ Request - vip_port_id: vip_port_id-optional - vip_qos_policy_id: vip_qos_policy_id-optional - vip_subnet_id: vip_subnet_id-optional + - vip_sg_ids: vip_sg_ids-optional Request Example ---------------- @@ -226,6 +235,7 @@ Response Parameters - vip_port_id: vip_port_id - vip_qos_policy_id: vip_qos_policy_id - vip_subnet_id: vip_subnet_id + - vip_sg_ids: vip_sg_ids - vip_vnic_type: vip_vnic_type Response Example @@ -322,6 +332,7 @@ Response Parameters - vip_port_id: vip_port_id - vip_qos_policy_id: vip_qos_policy_id - vip_subnet_id: vip_subnet_id + - vip_sg_ids: vip_sg_ids - vip_vnic_type: vip_vnic_type Response Example @@ -410,6 +421,7 @@ Response Parameters - vip_port_id: vip_port_id - vip_qos_policy_id: vip_qos_policy_id - vip_subnet_id: vip_subnet_id + - vip_sg_ids: vip_sg_ids - vip_vnic_type: vip_vnic_type Response Example diff --git a/doc/source/contributor/guides/providers.rst b/doc/source/contributor/guides/providers.rst index 8ff1ad7218..e1c4b79e22 100644 --- a/doc/source/contributor/guides/providers.rst +++ b/doc/source/contributor/guides/providers.rst @@ -161,6 +161,9 @@ contain the following: +-------------------+--------+-----------------------------------------------+ | vip_subnet_id | string | The ID of the subnet for the VIP. | +-------------------+--------+-----------------------------------------------+ +| vip_sg_ids | list | The list of Neutron Security Group IDs of the | +| | | VIP port (optional) | ++-------------------+--------+-----------------------------------------------+ The driver is expected to validate that the driver supports the request and raise an exception if the request cannot be accepted. @@ -195,6 +198,9 @@ dictionary. +-----------------+--------+-----------------------------------------------+ | vip_subnet_id | string | The ID of the subnet for the VIP. | +-----------------+--------+-----------------------------------------------+ +| vip_sg_ids | list | The list of Neutron Security Group IDs of the | +| | | VIP port (optional) | ++-----------------+--------+-----------------------------------------------+ **Creating a Fully Populated Load Balancer** diff --git a/doc/source/user/feature-classification/feature-matrix-lb.ini b/doc/source/user/feature-classification/feature-matrix-lb.ini index f24776665c..289a0393d1 100644 --- a/doc/source/user/feature-classification/feature-matrix-lb.ini +++ b/doc/source/user/feature-classification/feature-matrix-lb.ini @@ -128,3 +128,11 @@ status=optional cli=openstack loadbalancer create [--vip-subnet-id ] driver.amphora=complete driver.ovn=complete + +[operation.vip_sg_id] +title=vip_sg_id +status=optional +note=Optional Security Group of the VIP port (can be set multiple times). +cli=openstack loadbalancer create [--vip-sg-id ] +driver.amphora=complete +driver.ovn=missing diff --git a/octavia/api/root_controller.py b/octavia/api/root_controller.py index 393e3a9515..a7b269bddf 100644 --- a/octavia/api/root_controller.py +++ b/octavia/api/root_controller.py @@ -149,6 +149,9 @@ class RootController: self._add_a_version(versions, 'v2.27', 'v2', 'SUPPORTED', '2023-05-05T00:00:00Z', host_url) # Add port vnic_type for SR-IOV - self._add_a_version(versions, 'v2.28', 'v2', 'CURRENT', + self._add_a_version(versions, 'v2.28', 'v2', 'SUPPORTED', '2023-11-08T00:00:00Z', host_url) + # Add VIP SGs + self._add_a_version(versions, 'v2.29', 'v2', 'CURRENT', + '2024-10-15T00:00:00Z', host_url) return {'versions': versions} diff --git a/octavia/api/v2/controllers/listener.py b/octavia/api/v2/controllers/listener.py index 26077f03b9..51bc5f2e3c 100644 --- a/octavia/api/v2/controllers/listener.py +++ b/octavia/api/v2/controllers/listener.py @@ -157,7 +157,15 @@ class ListenersController(base.BaseController): value=headers, option=f'{listener_protocol} protocol listener.') - def _validate_cidr_compatible_with_vip(self, vips, allowed_cidrs): + def _validate_cidr_compatible_with_vip(self, db_vip: data_models.Vip, + vips: list[str], + allowed_cidrs: list[str]): + if allowed_cidrs and db_vip.sg_ids: + msg = _("Allowed CIDRs are not allowed when using custom VIP " + "Security Groups.") + raise exceptions.ValidationException( + detail=msg) + for cidr in allowed_cidrs: for vip in vips: # Check if CIDR IP version matches VIP IP version @@ -315,7 +323,8 @@ class ListenersController(base.BaseController): lock_session, id=lb_id) vip_addresses = [lb_db.vip.ip_address] vip_addresses.extend([vip.ip_address for vip in lb_db.additional_vips]) - self._validate_cidr_compatible_with_vip(vip_addresses, allowed_cidrs) + self._validate_cidr_compatible_with_vip(lb_db.vip, + vip_addresses, allowed_cidrs) if _can_tls_offload: # Validate TLS version list @@ -542,6 +551,7 @@ class ListenersController(base.BaseController): for vip in db_listener.load_balancer.additional_vips] ) self._validate_cidr_compatible_with_vip( + db_listener.load_balancer.vip, vip_addresses, listener.allowed_cidrs) # Check TLS cipher prohibit list diff --git a/octavia/api/v2/controllers/load_balancer.py b/octavia/api/v2/controllers/load_balancer.py index cdc876a6a9..c5019a32c6 100644 --- a/octavia/api/v2/controllers/load_balancer.py +++ b/octavia/api/v2/controllers/load_balancer.py @@ -307,6 +307,19 @@ class LoadBalancersController(base.BaseController): # Multi-vip validation for ensuring subnets are "sane" self._validate_subnets_share_network_but_no_duplicates(load_balancer) + # Validate optional security groups + if load_balancer.vip_sg_ids: + for sg_id in load_balancer.vip_sg_ids: + validate.security_group_exists(sg_id, context=context) + + def _validate_vnic_type(self, vnic_type: str, + load_balancer: lb_types.LoadBalancerPOST): + if (vnic_type == constants.VNIC_TYPE_DIRECT and + load_balancer.vip_sg_ids): + msg = _("VIP Security Groups are not allowed with VNIC direct " + "type") + raise exceptions.ValidationException(detail=msg) + @staticmethod def _create_vip_port_if_not_exist(load_balancer_db): """Create vip port.""" @@ -433,7 +446,7 @@ class LoadBalancersController(base.BaseController): @wsme_pecan.wsexpose(lb_types.LoadBalancerFullRootResponse, body=lb_types.LoadBalancerRootPOST, status_code=201) - def post(self, load_balancer): + def post(self, load_balancer: lb_types.LoadBalancerRootPOST): """Creates a load balancer.""" load_balancer = load_balancer.loadbalancer context = pecan_request.context.get('octavia_context') @@ -503,6 +516,9 @@ class LoadBalancersController(base.BaseController): else: vip_dict[constants.VNIC_TYPE] = constants.VNIC_TYPE_NORMAL + self._validate_vnic_type(vip_dict[constants.VNIC_TYPE], + load_balancer) + db_lb = self.repositories.create_load_balancer_and_vip( lock_session, lb_dict, vip_dict, additional_vip_dicts) @@ -724,6 +740,15 @@ class LoadBalancersController(base.BaseController): if db_lb.vip.qos_policy_id != load_balancer.vip_qos_policy_id: validate.qos_policy_exists(load_balancer.vip_qos_policy_id) + if not isinstance(load_balancer.vip_sg_ids, wtypes.UnsetType): + if load_balancer.vip_sg_ids is None: + load_balancer.vip_sg_ids = [] + else: + for sg_id in load_balancer.vip_sg_ids: + validate.security_group_exists(sg_id, context=context) + + self._validate_vnic_type(db_lb.vip.vnic_type, load_balancer) + # Load the driver early as it also provides validation driver = driver_factory.get_driver(db_lb.provider) diff --git a/octavia/api/v2/types/load_balancer.py b/octavia/api/v2/types/load_balancer.py index 4a1637d235..012d7d24e7 100644 --- a/octavia/api/v2/types/load_balancer.py +++ b/octavia/api/v2/types/load_balancer.py @@ -26,6 +26,7 @@ class BaseLoadBalancerType(types.BaseType): 'vip_network_id': 'vip.network_id', 'vip_qos_policy_id': 'vip.qos_policy_id', 'vip_vnic_type': 'vip.vnic_type', + 'vip_sg_ids': 'vip.sg_ids', 'admin_state_up': 'enabled'} _child_map = {'vip': { 'ip_address': 'vip_address', @@ -33,7 +34,8 @@ class BaseLoadBalancerType(types.BaseType): 'port_id': 'vip_port_id', 'network_id': 'vip_network_id', 'qos_policy_id': 'vip_qos_policy_id', - 'vnic_type': 'vip_vnic_type'}} + 'vnic_type': 'vip_vnic_type', + 'sg_ids': 'vip_sg_ids'}} class AdditionalVipsType(types.BaseType): @@ -57,6 +59,7 @@ class LoadBalancerResponse(BaseLoadBalancerType): vip_port_id = wtypes.wsattr(wtypes.UuidType()) vip_subnet_id = wtypes.wsattr(wtypes.UuidType()) vip_network_id = wtypes.wsattr(wtypes.UuidType()) + vip_sg_ids = wtypes.wsattr([wtypes.UuidType()]) additional_vips = wtypes.wsattr([AdditionalVipsType]) listeners = wtypes.wsattr([types.IdOnlyType]) pools = wtypes.wsattr([types.IdOnlyType]) @@ -78,6 +81,7 @@ class LoadBalancerResponse(BaseLoadBalancerType): result.vip_network_id = data_model.vip.network_id result.vip_qos_policy_id = data_model.vip.qos_policy_id result.vip_vnic_type = data_model.vip.vnic_type + result.vip_sg_ids = data_model.vip.sg_ids result.additional_vips = [ AdditionalVipsType.from_data_model(i) for i in data_model.additional_vips] @@ -131,6 +135,7 @@ class LoadBalancerPOST(BaseLoadBalancerType): vip_subnet_id = wtypes.wsattr(wtypes.UuidType()) vip_network_id = wtypes.wsattr(wtypes.UuidType()) vip_qos_policy_id = wtypes.wsattr(wtypes.UuidType()) + vip_sg_ids = wtypes.wsattr([wtypes.UuidType()]) additional_vips = wtypes.wsattr([AdditionalVipsType], default=[]) project_id = wtypes.wsattr(wtypes.StringType(max_length=36)) listeners = wtypes.wsattr([listener.ListenerSingleCreate], default=[]) @@ -152,6 +157,7 @@ class LoadBalancerPUT(BaseLoadBalancerType): description = wtypes.wsattr(wtypes.StringType(max_length=255)) vip_qos_policy_id = wtypes.wsattr(wtypes.UuidType()) admin_state_up = wtypes.wsattr(bool) + vip_sg_ids = wtypes.wsattr([wtypes.UuidType()]) tags = wtypes.wsattr(wtypes.ArrayType(wtypes.StringType(max_length=255))) diff --git a/octavia/common/utils.py b/octavia/common/utils.py index ee34b659d7..6381165ab8 100644 --- a/octavia/common/utils.py +++ b/octavia/common/utils.py @@ -23,6 +23,7 @@ import hashlib import ipaddress import re import socket +import typing from oslo_config import cfg from oslo_log import log as logging @@ -30,6 +31,8 @@ from oslo_utils import excutils from stevedore import driver as stevedore_driver from octavia.common import constants +if typing.TYPE_CHECKING: + from octavia.network import base as network_base CONF = cfg.CONF @@ -61,7 +64,7 @@ def get_amphora_driver(): return amphora_driver -def get_network_driver(): +def get_network_driver() -> 'network_base.AbstractNetworkDriver': CONF.import_group('controller_worker', 'octavia.common.config') network_driver = stevedore_driver.DriverManager( namespace='octavia.network.drivers', diff --git a/octavia/common/validate.py b/octavia/common/validate.py index 6aaee44d37..e2c33bcdc7 100644 --- a/octavia/common/validate.py +++ b/octavia/common/validate.py @@ -21,6 +21,7 @@ Defined here so these can also be used at deeper levels than the API. import ipaddress import re +import typing from oslo_config import cfg from rfc3986 import uri_reference @@ -33,6 +34,9 @@ from octavia.common import exceptions from octavia.common import utils from octavia.i18n import _ +if typing.TYPE_CHECKING: + from octavia.common import context + CONF = cfg.CONF _ListenerPUT = 'octavia.api.v2.types.listener.ListenerPUT' @@ -382,6 +386,18 @@ def network_exists_optionally_contains_subnet(network_id, subnet_id=None, return network +def security_group_exists(sg_id: str, + context: 'context.RequestContext' = None): + """Raises an exception when a security group does not exist.""" + network_driver = utils.get_network_driver() + try: + network_driver.get_security_group_by_id(sg_id, + context=context) + except Exception as e: + raise exceptions.InvalidSubresource( + resource='Security Group', id=sg_id) from e + + def network_allowed_by_config(network_id, valid_networks=None): if CONF.networking.valid_vip_networks and not valid_networks: valid_networks = CONF.networking.valid_vip_networks diff --git a/octavia/tests/functional/api/v2/test_load_balancer.py b/octavia/tests/functional/api/v2/test_load_balancer.py index 960010a254..17cd78375a 100644 --- a/octavia/tests/functional/api/v2/test_load_balancer.py +++ b/octavia/tests/functional/api/v2/test_load_balancer.py @@ -2054,6 +2054,37 @@ class TestLoadBalancer(base.BaseAPITest): self.put(self.LB_PATH.format(lb_id=lb_dict.get('id')), lb_json, status=400) + def test_update_with_sg_ids(self): + project_id = uuidutils.generate_uuid() + sg1_id = uuidutils.generate_uuid() + sg2_id = uuidutils.generate_uuid() + lb = self.create_load_balancer(uuidutils.generate_uuid(), + name='lb1', + project_id=project_id, + vip_sg_ids=[sg1_id, sg2_id]) + lb_dict = lb.get(self.root_tag) + lb_id = lb_dict.get('id') + self.assertEqual(sorted([sg1_id, sg2_id]), + sorted(lb_dict['vip_sg_ids'])) + self.set_lb_status(lb_dict.get('id')) + + lb_json = self._build_body({'vip_sg_ids': [sg2_id]}) + self.put(self.LB_PATH.format(lb_id=lb_id), + lb_json, status=200) + self.set_lb_status(lb_dict.get('id')) + response = self.get(self.LB_PATH.format(lb_id=lb_id)) + lb_dict = response.json.get(self.root_tag) + self.assertEqual(sorted([sg2_id]), + sorted(lb_dict['vip_sg_ids'])) + + lb_json = self._build_body({'vip_sg_ids': []}) + lb = self.put(self.LB_PATH.format(lb_id=lb_dict.get('id')), + lb_json, status=200) + self.set_lb_status(lb_dict.get('id')) + response = self.get(self.LB_PATH.format(lb_id=lb_id)) + lb_dict = response.json.get(self.root_tag) + self.assertEqual(0, len(lb_dict['vip_sg_ids'])) + def test_update_bad_lb_id(self): path = self.LB_PATH.format(lb_id='SEAN-CONNERY') self.put(path, body={}, status=404) @@ -2735,8 +2766,13 @@ class TestLoadBalancerGraph(base.BaseAPITest): expected_additional_vips = expected_graph.pop('additional_vips', []) observed_additional_vips = observed_graph_copy.pop('additional_vips', []) + expected_vip_sg_ids = expected_graph.pop('vip_sg_ids', []) + observed_vip_sg_ids = observed_graph_copy.pop('vip_sg_ids', []) self.assertEqual(expected_graph, observed_graph_copy) + self.assertEqual(sorted(expected_vip_sg_ids), + sorted(observed_vip_sg_ids)) + self.assertEqual(len(expected_pools), len(observed_pools)) self.assertEqual(len(expected_listeners), len(observed_listeners)) @@ -2800,7 +2836,8 @@ class TestLoadBalancerGraph(base.BaseAPITest): self.assertIn(observed_add_vip, expected_additional_vips) def _get_lb_bodies(self, create_listeners, expected_listeners, - create_pools=None, additional_vips=None): + create_pools=None, additional_vips=None, + vip_sg_ids=None): create_lb = { 'name': 'lb1', 'project_id': self._project_id, @@ -2811,6 +2848,8 @@ class TestLoadBalancerGraph(base.BaseAPITest): 'listeners': create_listeners, 'pools': create_pools or [] } + if vip_sg_ids: + create_lb['vip_sg_ids'] = vip_sg_ids if additional_vips: create_lb.update({'additional_vips': additional_vips}) expected_lb = { @@ -2830,6 +2869,7 @@ class TestLoadBalancerGraph(base.BaseAPITest): 'provider': 'noop_driver', 'tags': [], 'vip_vnic_type': constants.VNIC_TYPE_NORMAL, + 'vip_sg_ids': vip_sg_ids or [], } expected_lb.update(create_lb) expected_lb['listeners'] = expected_listeners @@ -3211,6 +3251,16 @@ class TestLoadBalancerGraph(base.BaseAPITest): api_lb = response.json.get(self.root_tag) self._assert_graphs_equal(expected_lb, api_lb) + def test_with_sg_ids(self): + create_lb, expected_lb = self._get_lb_bodies( + [], [], vip_sg_ids=[uuidutils.generate_uuid(), + uuidutils.generate_uuid()]) + + body = self._build_body(create_lb) + response = self.post(self.LBS_PATH, body) + api_lb = response.json.get(self.root_tag) + self._assert_graphs_equal(expected_lb, api_lb) + def test_with_one_listener(self): create_listener, expected_listener = self._get_listener_bodies() create_lb, expected_lb = self._get_lb_bodies([create_listener], @@ -3220,6 +3270,53 @@ class TestLoadBalancerGraph(base.BaseAPITest): api_lb = response.json.get(self.root_tag) self._assert_graphs_equal(expected_lb, api_lb) + def test_with_one_listener_sg_ids(self): + create_listener, expected_listener = self._get_listener_bodies() + create_lb, expected_lb = self._get_lb_bodies( + [create_listener], [expected_listener], + vip_sg_ids=[uuidutils.generate_uuid(), + uuidutils.generate_uuid()]) + + body = self._build_body(create_lb) + response = self.post(self.LBS_PATH, body) + api_lb = response.json.get(self.root_tag) + self._assert_graphs_equal(expected_lb, api_lb) + + @mock.patch('octavia.api.v2.controllers.load_balancer.' + 'LoadBalancersController._apply_flavor_to_lb_dict', + return_value={constants.SRIOV_VIP: True}) + def test_with_vip_vnic_type_direct_and_sg_ids(self, mock_flavor_dict): + create_lb, expected_lb = self._get_lb_bodies( + [], [], + vip_sg_ids=[uuidutils.generate_uuid(), + uuidutils.generate_uuid()]) + expected_lb[constants.VIP_VNIC_TYPE] = constants.VNIC_TYPE_DIRECT + + body = self._build_body(create_lb) + + response = self.post(self.LBS_PATH, body, status=400, + expect_errors=True) + error_text = response.json.get('faultstring') + self.assertIn("VIP Security Groups are not allowed with VNIC " + "direct type", error_text) + + def test_with_one_listener_sg_ids_and_allowed_cidrs(self): + allowed_cidrs = ['10.0.1.0/24'] + create_listener, expected_listener = self._get_listener_bodies( + create_allowed_cidrs=allowed_cidrs, + expected_allowed_cidrs=allowed_cidrs) + create_lb, expected_lb = self._get_lb_bodies( + [create_listener], [expected_listener], + vip_sg_ids=[uuidutils.generate_uuid(), + uuidutils.generate_uuid()]) + + body = self._build_body(create_lb) + response = self.post(self.LBS_PATH, body, status=400, + expect_errors=True) + error_text = response.json.get('faultstring') + self.assertIn("Allowed CIDRs are not allowed when using custom " + "VIP Security Groups", error_text) + def test_with_one_listener_with_default_timeouts(self): self.conf = self.useFixture(oslo_fixture.Config(cfg.CONF)) self.conf.config(group='haproxy_amphora', timeout_client_data=20) diff --git a/releasenotes/notes/add_vip_sg_ids-feaeaf8b3301e267.yaml b/releasenotes/notes/add_vip_sg_ids-feaeaf8b3301e267.yaml new file mode 100644 index 0000000000..3a609d257b --- /dev/null +++ b/releasenotes/notes/add_vip_sg_ids-feaeaf8b3301e267.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Add the ``vip_sg_ids`` parameter to the load-balancer POST API. It allows to + set a list of user-defined Neutron Security Groups on the VIP port of the + Load Balancer.