diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 2697e13426..fd30f84025 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -973,7 +973,7 @@ session_persistence-optional: session_persistence_cookie: description: | The name of the cookie to use for session persistence. Only applicable to - the ``APP_COOKIE`` session persistence type. + the ``APP_COOKIE`` session persistence type where it is required. in: body required: false type: string diff --git a/octavia/api/v2/controllers/pool.py b/octavia/api/v2/controllers/pool.py index cc06df750c..6ff06a9182 100644 --- a/octavia/api/v2/controllers/pool.py +++ b/octavia/api/v2/controllers/pool.py @@ -29,6 +29,7 @@ from octavia.api.v2.types import pool as pool_types from octavia.common import constants from octavia.common import data_models from octavia.common import exceptions +from octavia.common import validate from octavia.db import api as db_api from octavia.db import prepare as db_prepare from octavia.i18n import _ @@ -171,6 +172,10 @@ class PoolsController(base.BaseController): self._auth_validate_action(context, pool.project_id, constants.RBAC_POST) + if pool.session_persistence: + sp_dict = pool.session_persistence.to_dict(render_unsets=False) + validate.check_session_persistence(sp_dict) + lock_session = db_api.get_session(autocommit=False) try: if self.repositories.check_quota_met( @@ -253,12 +258,17 @@ class PoolsController(base.BaseController): self._auth_validate_action(context, db_pool.project_id, constants.RBAC_PUT) + if pool.session_persistence: + sp_dict = pool.session_persistence.to_dict(render_unsets=False) + validate.check_session_persistence(sp_dict) + self._test_lb_and_listener_statuses( context.session, lb_id=db_pool.load_balancer_id, listener_ids=self._get_affected_listener_ids(db_pool)) self.repositories.pool.update( context.session, db_pool.id, provisioning_status=constants.PENDING_UPDATE) + try: LOG.info("Sending Update of Pool %s to handler", id) self.handler.update(db_pool, pool) diff --git a/octavia/api/v2/types/pool.py b/octavia/api/v2/types/pool.py index 624e95516c..fd8079ddab 100644 --- a/octavia/api/v2/types/pool.py +++ b/octavia/api/v2/types/pool.py @@ -30,13 +30,15 @@ class SessionPersistencePOST(types.BaseType): """Defines mandatory and optional attributes of a POST request.""" type = wtypes.wsattr(wtypes.Enum(str, *constants.SUPPORTED_SP_TYPES), mandatory=True) - cookie_name = wtypes.wsattr(wtypes.text) + cookie_name = wtypes.wsattr(wtypes.StringType(max_length=255), + default=None) class SessionPersistencePUT(types.BaseType): """Defines attributes that are acceptable of a PUT request.""" type = wtypes.wsattr(wtypes.Enum(str, *constants.SUPPORTED_SP_TYPES)) - cookie_name = wtypes.wsattr(wtypes.text, default=None) + cookie_name = wtypes.wsattr(wtypes.StringType(max_length=255), + default=None) class BasePoolType(types.BaseType): diff --git a/octavia/common/validate.py b/octavia/common/validate.py index b2d17ec544..c4abdd2e02 100644 --- a/octavia/common/validate.py +++ b/octavia/common/validate.py @@ -293,3 +293,29 @@ def is_ip_member_of_cidr(address, cidr): if netaddr.IPAddress(address) in netaddr.IPNetwork(cidr): return True return False + + +def check_session_persistence(SP_dict): + try: + if SP_dict['cookie_name']: + if SP_dict['type'] != constants.SESSION_PERSISTENCE_APP_COOKIE: + raise exceptions.ValidationException(detail=_( + 'Field "cookie_name" can only be specified with session ' + 'persistence of type "APP_COOKIE".')) + bad_cookie_name = re.compile(r'[\x00-\x20\x22\x28-\x29\x2c\x2f' + r'\x3a-\x40\x5b-\x5d\x7b\x7d\x7f]+') + valid_chars = re.compile(r'[\x00-\xff]+') + if (bad_cookie_name.search(SP_dict['cookie_name']) or + not valid_chars.search(SP_dict['cookie_name'])): + raise exceptions.ValidationException(detail=_( + 'Supplied "cookie_name" is invalid.')) + if (SP_dict['type'] == constants.SESSION_PERSISTENCE_APP_COOKIE and + not SP_dict['cookie_name']): + raise exceptions.ValidationException(detail=_( + 'Field "cookie_name" must be specified when using the ' + '"APP_COOKIE" session persistence type.')) + except exceptions.ValidationException: + raise + except Exception: + raise exceptions.ValidationException(detail=_( + 'Invalid session_persistence provided.')) diff --git a/octavia/tests/functional/api/v2/test_pool.py b/octavia/tests/functional/api/v2/test_pool.py index d57cee76c8..eb5f0596e6 100644 --- a/octavia/tests/functional/api/v2/test_pool.py +++ b/octavia/tests/functional/api/v2/test_pool.py @@ -1018,7 +1018,7 @@ class TestPool(base.BaseAPITest): pool_prov_status=constants.ERROR) def test_create_with_session_persistence(self): - sp = {"type": constants.SESSION_PERSISTENCE_HTTP_COOKIE, + sp = {"type": constants.SESSION_PERSISTENCE_APP_COOKIE, "cookie_name": "test_cookie_name"} optionals = {"listener_id": self.listener_id, "session_persistence": sp} @@ -1039,7 +1039,7 @@ class TestPool(base.BaseAPITest): pool_id=api_pool.get('id'))).json.get(self.root_tag) sess_p = response.get('session_persistence') self.assertIsNotNone(sess_p) - self.assertEqual(constants.SESSION_PERSISTENCE_HTTP_COOKIE, + self.assertEqual(constants.SESSION_PERSISTENCE_APP_COOKIE, sess_p.get('type')) self.assertEqual('test_cookie_name', sess_p.get('cookie_name')) self.assert_correct_status( @@ -1057,9 +1057,52 @@ class TestPool(base.BaseAPITest): 'session_persistence': sp} self.post(self.POOLS_PATH, self._build_body(lb_pool), status=400) - def test_add_session_persistence(self): + def test_create_with_bad_SP_type_HTTP_cookie(self): sp = {"type": constants.SESSION_PERSISTENCE_HTTP_COOKIE, "cookie_name": "test_cookie_name"} + lb_pool = { + 'loadbalancer_id': self.lb_id, + 'listener_id': self.listener_id, + 'protocol': constants.PROTOCOL_HTTP, + 'lb_algorithm': constants.LB_ALGORITHM_ROUND_ROBIN, + 'session_persistence': sp} + self.post(self.POOLS_PATH, self._build_body(lb_pool), status=400) + + def test_create_with_bad_SP_type_IP_cookie(self): + sp = {"type": constants.SESSION_PERSISTENCE_SOURCE_IP, + "cookie_name": "test_cookie_name"} + lb_pool = { + 'loadbalancer_id': self.lb_id, + 'listener_id': self.listener_id, + 'protocol': constants.PROTOCOL_HTTP, + 'lb_algorithm': constants.LB_ALGORITHM_ROUND_ROBIN, + 'session_persistence': sp} + self.post(self.POOLS_PATH, self._build_body(lb_pool), status=400) + + def test_create_with_bad_SP_cookie_name(self): + sp = {"type": constants.SESSION_PERSISTENCE_APP_COOKIE, + "cookie_name": "b@d_cookie_name"} + lb_pool = { + 'loadbalancer_id': self.lb_id, + 'listener_id': self.listener_id, + 'protocol': constants.PROTOCOL_HTTP, + 'lb_algorithm': constants.LB_ALGORITHM_ROUND_ROBIN, + 'session_persistence': sp} + self.post(self.POOLS_PATH, self._build_body(lb_pool), status=400) + + def test_create_with_missing_cookie_name(self): + sp = {"type": constants.SESSION_PERSISTENCE_APP_COOKIE} + lb_pool = { + 'loadbalancer_id': self.lb_id, + 'listener_id': self.listener_id, + 'protocol': constants.PROTOCOL_HTTP, + 'lb_algorithm': constants.LB_ALGORITHM_ROUND_ROBIN, + 'session_persistence': sp} + self.post(self.POOLS_PATH, self._build_body(lb_pool), status=400) + + def test_add_session_persistence(self): + sp = {"type": constants.SESSION_PERSISTENCE_APP_COOKIE, + "cookie_name": "test_cookie_name"} api_pool = self.create_pool( self.lb_id, constants.PROTOCOL_HTTP, @@ -1080,7 +1123,7 @@ class TestPool(base.BaseAPITest): pool_prov_status=constants.PENDING_UPDATE) def test_update_session_persistence(self): - sp = {"type": constants.SESSION_PERSISTENCE_HTTP_COOKIE, + sp = {"type": constants.SESSION_PERSISTENCE_APP_COOKIE, "cookie_name": "test_cookie_name"} optionals = {"listener_id": self.listener_id, "session_persistence": sp} @@ -1109,7 +1152,7 @@ class TestPool(base.BaseAPITest): pool_prov_status=constants.PENDING_UPDATE) def test_update_preserve_session_persistence(self): - sp = {"type": constants.SESSION_PERSISTENCE_HTTP_COOKIE, + sp = {"type": constants.SESSION_PERSISTENCE_APP_COOKIE, "cookie_name": "test_cookie_name"} optionals = {"listener_id": self.listener_id, "name": "name", "session_persistence": sp} @@ -1133,7 +1176,7 @@ class TestPool(base.BaseAPITest): pool_prov_status=constants.PENDING_UPDATE) def test_update_bad_session_persistence(self): - sp = {"type": constants.SESSION_PERSISTENCE_HTTP_COOKIE, + sp = {"type": constants.SESSION_PERSISTENCE_APP_COOKIE, "cookie_name": "test_cookie_name"} optionals = {"listener_id": self.listener_id, "session_persistence": sp} @@ -1151,8 +1194,83 @@ class TestPool(base.BaseAPITest): self.put(self.POOL_PATH.format(pool_id=api_pool.get('id')), self._build_body(new_pool), status=400) + def test_update_with_bad_SP_type_HTTP_cookie(self): + sp = {"type": constants.SESSION_PERSISTENCE_SOURCE_IP} + optionals = {"listener_id": self.listener_id, + "session_persistence": sp} + api_pool = self.create_pool( + self.lb_id, + constants.PROTOCOL_HTTP, + constants.LB_ALGORITHM_ROUND_ROBIN, + **optionals).get(self.root_tag) + self.set_lb_status(lb_id=self.lb_id) + response = self.get(self.POOL_PATH.format( + pool_id=api_pool.get('id'))).json.get(self.root_tag) + sess_p = response.get('session_persistence') + sess_p['type'] = constants.SESSION_PERSISTENCE_HTTP_COOKIE + sess_p['cookie_name'] = 'test_cookie_name' + new_pool = {'session_persistence': sess_p} + self.put(self.POOL_PATH.format(pool_id=api_pool.get('id')), + self._build_body(new_pool), status=400) + + def test_update_with_bad_SP_type_IP_cookie(self): + sp = {"type": constants.SESSION_PERSISTENCE_HTTP_COOKIE} + optionals = {"listener_id": self.listener_id, + "session_persistence": sp} + api_pool = self.create_pool( + self.lb_id, + constants.PROTOCOL_HTTP, + constants.LB_ALGORITHM_ROUND_ROBIN, + **optionals).get(self.root_tag) + self.set_lb_status(lb_id=self.lb_id) + response = self.get(self.POOL_PATH.format( + pool_id=api_pool.get('id'))).json.get(self.root_tag) + sess_p = response.get('session_persistence') + sess_p['type'] = constants.SESSION_PERSISTENCE_SOURCE_IP + sess_p['cookie_name'] = 'test_cookie_name' + new_pool = {'session_persistence': sess_p} + self.put(self.POOL_PATH.format(pool_id=api_pool.get('id')), + self._build_body(new_pool), status=400) + + def test_update_with_bad_SP_cookie_name(self): + sp = {"type": constants.SESSION_PERSISTENCE_SOURCE_IP} + optionals = {"listener_id": self.listener_id, + "session_persistence": sp} + api_pool = self.create_pool( + self.lb_id, + constants.PROTOCOL_HTTP, + constants.LB_ALGORITHM_ROUND_ROBIN, + **optionals).get(self.root_tag) + self.set_lb_status(lb_id=self.lb_id) + response = self.get(self.POOL_PATH.format( + pool_id=api_pool.get('id'))).json.get(self.root_tag) + sess_p = response.get('session_persistence') + sess_p['type'] = constants.SESSION_PERSISTENCE_APP_COOKIE + sess_p['cookie_name'] = 'b@d_cookie_name' + new_pool = {'session_persistence': sess_p} + self.put(self.POOL_PATH.format(pool_id=api_pool.get('id')), + self._build_body(new_pool), status=400) + + def test_update_with_missing_SP_cookie_name(self): + sp = {"type": constants.SESSION_PERSISTENCE_SOURCE_IP} + optionals = {"listener_id": self.listener_id, + "session_persistence": sp} + api_pool = self.create_pool( + self.lb_id, + constants.PROTOCOL_HTTP, + constants.LB_ALGORITHM_ROUND_ROBIN, + **optionals).get(self.root_tag) + self.set_lb_status(lb_id=self.lb_id) + response = self.get(self.POOL_PATH.format( + pool_id=api_pool.get('id'))).json.get(self.root_tag) + sess_p = response.get('session_persistence') + sess_p['type'] = constants.SESSION_PERSISTENCE_APP_COOKIE + new_pool = {'session_persistence': sess_p} + self.put(self.POOL_PATH.format(pool_id=api_pool.get('id')), + self._build_body(new_pool), status=400) + def test_delete_with_session_persistence(self): - sp = {"type": constants.SESSION_PERSISTENCE_HTTP_COOKIE, + sp = {"type": constants.SESSION_PERSISTENCE_APP_COOKIE, "cookie_name": "test_cookie_name"} optionals = {"listener_id": self.listener_id, "session_persistence": sp} @@ -1171,7 +1289,7 @@ class TestPool(base.BaseAPITest): pool_prov_status=constants.PENDING_DELETE) def test_delete_session_persistence(self): - sp = {"type": constants.SESSION_PERSISTENCE_HTTP_COOKIE, + sp = {"type": constants.SESSION_PERSISTENCE_APP_COOKIE, "cookie_name": "test_cookie_name"} optionals = {"listener_id": self.listener_id, "session_persistence": sp} diff --git a/octavia/tests/unit/common/test_validations.py b/octavia/tests/unit/common/test_validations.py index e0d57b8311..a80330ea92 100644 --- a/octavia/tests/unit/common/test_validations.py +++ b/octavia/tests/unit/common/test_validations.py @@ -373,3 +373,44 @@ class TestValidations(base.TestCase): self.assertRaises(exceptions.InvalidSubresource, validate.qos_policy_exists, qos_policy_id) + + def test_check_session_persistence(self): + valid_cookie_name_dict = {'type': 'APP_COOKIE', + 'cookie_name': 'chocolate_chip'} + invalid_cookie_name_dict = {'type': 'APP_COOKIE', + 'cookie_name': '@chocolate_chip'} + invalid_type_HTTP_cookie_name_dict = {'type': 'HTTP_COOKIE', + 'cookie_name': 'chocolate_chip'} + invalid_type_IP_cookie_name_dict = {'type': 'SOURCE_IP', + 'cookie_name': 'chocolate_chip'} + invalid_missing_cookie_name_dict = {'type': 'APP_COOKIE'} + + # Validate that a good cookie name passes + validate.check_session_persistence(valid_cookie_name_dict) + + # Test raises with providing an invalid cookie name + self.assertRaises(exceptions.ValidationException, + validate.check_session_persistence, + invalid_cookie_name_dict) + + # Test raises type HTTP_COOKIE and providing cookie_name + self.assertRaises(exceptions.ValidationException, + validate.check_session_persistence, + invalid_type_HTTP_cookie_name_dict) + + # Test raises type SOURCE_IP and providing cookie_name + self.assertRaises(exceptions.ValidationException, + validate.check_session_persistence, + invalid_type_IP_cookie_name_dict) + + # Test raises when type APP_COOKIE but no cookie_name + self.assertRaises(exceptions.ValidationException, + validate.check_session_persistence, + invalid_missing_cookie_name_dict) + + # Test catch all exception raises a user friendly message + with mock.patch('re.compile') as compile_mock: + compile_mock.side_effect = Exception + self.assertRaises(exceptions.ValidationException, + validate.check_session_persistence, + valid_cookie_name_dict)