diff --git a/octavia/api/v2/controllers/health_monitor.py b/octavia/api/v2/controllers/health_monitor.py index 096a99469c..cf49c37e02 100644 --- a/octavia/api/v2/controllers/health_monitor.py +++ b/octavia/api/v2/controllers/health_monitor.py @@ -92,6 +92,9 @@ class HealthMonitorController(base.BaseController): # mutable pool = self.repositories.pool.get(session, id=hm.pool_id) load_balancer_id = pool.load_balancer_id + # Check the parent is not locked for some reason (ERROR, etc.) + if pool.provisioning_status not in consts.MUTABLE_STATUSES: + raise exceptions.ImmutableObject(resource='Pool', id=hm.pool_id) if not self.repositories.test_and_set_lb_and_listeners_prov_status( session, load_balancer_id, consts.PENDING_UPDATE, consts.PENDING_UPDATE, diff --git a/octavia/api/v2/controllers/l7rule.py b/octavia/api/v2/controllers/l7rule.py index 722cfeb1e8..f37f794193 100644 --- a/octavia/api/v2/controllers/l7rule.py +++ b/octavia/api/v2/controllers/l7rule.py @@ -87,6 +87,10 @@ class L7RuleController(base.BaseController): l7policy = self._get_db_l7policy(session, self.l7policy_id) listener_id = l7policy.listener_id load_balancer_id = l7policy.listener.load_balancer_id + # Check the parent is not locked for some reason (ERROR, etc.) + if l7policy.provisioning_status not in constants.MUTABLE_STATUSES: + raise exceptions.ImmutableObject(resource='L7Policy', + id=self.l7policy_id) if not self.repositories.test_and_set_lb_and_listeners_prov_status( session, load_balancer_id, constants.PENDING_UPDATE, constants.PENDING_UPDATE, diff --git a/octavia/api/v2/controllers/member.py b/octavia/api/v2/controllers/member.py index c9288b8a9f..ddf3602cc8 100644 --- a/octavia/api/v2/controllers/member.py +++ b/octavia/api/v2/controllers/member.py @@ -54,6 +54,8 @@ class MemberController(base.BaseController): self._auth_validate_action(context, db_member.project_id, constants.RBAC_GET_ONE) + self._validate_pool_id(id, db_member.pool_id) + result = self._convert_db_to_type( db_member, member_types.MemberResponse) if fields is not None: @@ -98,6 +100,9 @@ class MemberController(base.BaseController): # We need to verify that any listeners referencing this member's # pool are also mutable pool = self._get_db_pool(session, self.pool_id) + # Check the parent is not locked for some reason (ERROR, etc.) + if pool.provisioning_status not in constants.MUTABLE_STATUSES: + raise exceptions.ImmutableObject(resource='Pool', id=self.pool_id) load_balancer_id = pool.load_balancer_id if not self.repositories.test_and_set_lb_and_listeners_prov_status( session, load_balancer_id, @@ -129,6 +134,10 @@ class MemberController(base.BaseController): # do not give any information as to what constraint failed raise exceptions.InvalidOption(value='', option='') + def _validate_pool_id(self, member_id, db_member_pool_id): + if db_member_pool_id != self.pool_id: + raise exceptions.NotFound(resource='Member', id=member_id) + @wsme_pecan.wsexpose(member_types.MemberRootResponse, body=member_types.MemberRootPOST, status_code=201) def post(self, member_): @@ -214,6 +223,8 @@ class MemberController(base.BaseController): self._auth_validate_action(context, project_id, constants.RBAC_PUT) + self._validate_pool_id(id, db_member.pool_id) + # Load the driver early as it also provides validation driver = driver_factory.get_driver(provider) @@ -266,6 +277,8 @@ class MemberController(base.BaseController): self._auth_validate_action(context, project_id, constants.RBAC_DELETE) + self._validate_pool_id(id, db_member.pool_id) + # Load the driver early as it also provides validation driver = driver_factory.get_driver(provider) diff --git a/octavia/controller/worker/task_utils.py b/octavia/controller/worker/task_utils.py index 44b2cda964..7f78034067 100644 --- a/octavia/controller/worker/task_utils.py +++ b/octavia/controller/worker/task_utils.py @@ -87,6 +87,22 @@ class TaskUtils(object): "provisioning status to ERROR due to: " "%(except)s", {'health': health_mon_id, 'except': e}) + def mark_l7policy_prov_status_active(self, l7policy_id): + """Sets a L7 policy provisioning status to ACTIVE. + + NOTE: This should only be called from revert methods. + + :param l7policy_id: L7 Policy ID to set provisioning status to ACTIVE + """ + try: + self.l7policy_repo.update(db_apis.get_session(), + id=l7policy_id, + provisioning_status=constants.ACTIVE) + except Exception as e: + LOG.error("Failed to update l7policy %(l7p)s " + "provisioning status to ACTIVE due to: " + "%(except)s", {'l7p': l7policy_id, 'except': e}) + def mark_l7policy_prov_status_error(self, l7policy_id): """Sets a L7 policy provisioning status to ERROR. diff --git a/octavia/controller/worker/tasks/lifecycle_tasks.py b/octavia/controller/worker/tasks/lifecycle_tasks.py index d199e5dd26..291b5677c5 100644 --- a/octavia/controller/worker/tasks/lifecycle_tasks.py +++ b/octavia/controller/worker/tasks/lifecycle_tasks.py @@ -53,6 +53,7 @@ class HealthMonitorToErrorOnRevertTask(BaseLifecycleTask): def revert(self, health_mon, listeners, loadbalancer, *args, **kwargs): self.task_utils.mark_health_mon_prov_status_error(health_mon.pool_id) + self.task_utils.mark_pool_prov_status_active(health_mon.pool_id) self.task_utils.mark_loadbalancer_prov_status_active(loadbalancer.id) for listener in listeners: self.task_utils.mark_listener_prov_status_active(listener.id) @@ -79,6 +80,7 @@ class L7RuleToErrorOnRevertTask(BaseLifecycleTask): def revert(self, l7rule, listeners, loadbalancer, *args, **kwargs): self.task_utils.mark_l7rule_prov_status_error(l7rule.id) + self.task_utils.mark_l7policy_prov_status_active(l7rule.l7policy_id) self.task_utils.mark_loadbalancer_prov_status_active(loadbalancer.id) for listener in listeners: self.task_utils.mark_listener_prov_status_active(listener.id) diff --git a/octavia/tests/functional/api/v2/test_health_monitor.py b/octavia/tests/functional/api/v2/test_health_monitor.py index 78971e9c5f..249455aa3a 100644 --- a/octavia/tests/functional/api/v2/test_health_monitor.py +++ b/octavia/tests/functional/api/v2/test_health_monitor.py @@ -22,6 +22,7 @@ from octavia.common import constants import octavia.common.context from octavia.common import data_models from octavia.common import exceptions +from octavia.db import repositories from octavia.tests.functional.api.v2 import base @@ -53,6 +54,7 @@ class TestHealthMonitor(base.BaseAPITest): self.pool_with_listener_id = ( self.pool_with_listener.get('pool').get('id')) self.set_lb_status(self.lb_id) + self.pool_repo = repositories.PoolRepository() self._setup_udp_lb_resources() def _setup_udp_lb_resources(self): @@ -999,6 +1001,24 @@ class TestHealthMonitor(base.BaseAPITest): self.conf.config(group='api_settings', auth_strategy=auth_strategy) self.assertEqual(self.NOT_AUTHORIZED_BODY, api_hm) + def test_create_pool_in_error(self): + project_id = uuidutils.generate_uuid() + lb1 = self.create_load_balancer(uuidutils.generate_uuid(), name='lb1', + project_id=project_id) + lb1_id = lb1.get('loadbalancer').get('id') + self.set_lb_status(lb1_id) + pool1 = self.create_pool( + lb1_id, constants.PROTOCOL_HTTP, + constants.LB_ALGORITHM_ROUND_ROBIN).get('pool') + pool1_id = pool1.get('id') + self.set_lb_status(lb1_id) + self.set_object_status(self.pool_repo, pool1_id, + provisioning_status=constants.ERROR) + api_hm = self.create_health_monitor( + pool1_id, constants.HEALTH_MONITOR_HTTP, 1, 1, 1, 1, status=409) + ref_msg = 'Pool %s is immutable and cannot be updated.' % pool1_id + self.assertEqual(ref_msg, api_hm.get('faultstring')) + def test_create_with_listener(self): api_hm = self.create_health_monitor( self.pool_with_listener_id, constants.HEALTH_MONITOR_HTTP, diff --git a/octavia/tests/functional/api/v2/test_l7rule.py b/octavia/tests/functional/api/v2/test_l7rule.py index 46a19a7fbf..0ba838f2d9 100644 --- a/octavia/tests/functional/api/v2/test_l7rule.py +++ b/octavia/tests/functional/api/v2/test_l7rule.py @@ -20,6 +20,7 @@ from oslo_utils import uuidutils from octavia.common import constants import octavia.common.context from octavia.common import exceptions +from octavia.db import repositories from octavia.tests.functional.api.v2 import base @@ -46,6 +47,7 @@ class TestL7Rule(base.BaseAPITest): self.l7rules_path = self.L7RULES_PATH.format( l7policy_id=self.l7policy_id) self.l7rule_path = self.l7rules_path + '/{l7rule_id}' + self.l7policy_repo = repositories.L7PolicyRepository() def test_get(self): l7rule = self.create_l7rule( @@ -541,6 +543,21 @@ class TestL7Rule(base.BaseAPITest): self.conf.config(group='api_settings', auth_strategy=auth_strategy) self.assertEqual(self.NOT_AUTHORIZED_BODY, api_l7rule) + def test_create_l7policy_in_error(self): + l7policy = self.create_l7policy( + self.listener_id, constants.L7POLICY_ACTION_REJECT) + l7policy_id = l7policy.get('l7policy').get('id') + self.set_lb_status(self.lb_id) + self.set_object_status(self.l7policy_repo, l7policy_id, + provisioning_status=constants.ERROR) + api_l7rule = self.create_l7rule( + l7policy_id, constants.L7RULE_TYPE_HOST_NAME, + constants.L7RULE_COMPARE_TYPE_EQUAL_TO, + 'www.example.com', status=409) + ref_msg = ('L7Policy %s is immutable and cannot be updated.' % + l7policy_id) + self.assertEqual(ref_msg, api_l7rule.get('faultstring')) + def test_create_path_rule(self): api_l7rule = self.create_l7rule( self.l7policy_id, constants.L7RULE_TYPE_PATH, diff --git a/octavia/tests/functional/api/v2/test_member.py b/octavia/tests/functional/api/v2/test_member.py index 21b3ab299d..2124fd6eb8 100644 --- a/octavia/tests/functional/api/v2/test_member.py +++ b/octavia/tests/functional/api/v2/test_member.py @@ -23,6 +23,7 @@ from octavia.common import constants import octavia.common.context from octavia.common import data_models from octavia.common import exceptions +from octavia.db import repositories from octavia.network import base as network_base from octavia.tests.functional.api.v2 import base @@ -61,6 +62,7 @@ class TestMember(base.BaseAPITest): self.members_path_listener = self.MEMBERS_PATH.format( pool_id=self.pool_with_listener_id) self.member_path_listener = self.members_path_listener + '/{member_id}' + self.pool_repo = repositories.PoolRepository() def test_get(self): api_member = self.create_member( @@ -527,6 +529,23 @@ class TestMember(base.BaseAPITest): self.conf.config(group='api_settings', auth_strategy=auth_strategy) self.assertEqual(self.NOT_AUTHORIZED_BODY, api_member) + def test_create_pool_in_error(self): + project_id = uuidutils.generate_uuid() + lb1 = self.create_load_balancer(uuidutils.generate_uuid(), name='lb1', + project_id=project_id) + lb1_id = lb1.get('loadbalancer').get('id') + self.set_lb_status(lb1_id) + pool1 = self.create_pool( + lb1_id, constants.PROTOCOL_HTTP, + constants.LB_ALGORITHM_ROUND_ROBIN).get('pool') + pool1_id = pool1.get('id') + self.set_lb_status(lb1_id) + self.set_object_status(self.pool_repo, pool1_id, + provisioning_status=constants.ERROR) + api_member = self.create_member(pool1_id, '192.0.2.1', 80, status=409) + ref_msg = 'Pool %s is immutable and cannot be updated.' % pool1_id + self.assertEqual(ref_msg, api_member.get('faultstring')) + # TODO(rm_work) Remove after deprecation of project_id in POST (R series) def test_create_with_project_id_is_ignored(self): pid = uuidutils.generate_uuid() @@ -1162,6 +1181,23 @@ class TestMember(base.BaseAPITest): self.delete(self.member_path.format( member_id=uuidutils.generate_uuid()), status=404) + def test_delete_mismatch_pool(self): + # Create a pool that will not have the member, but is valid. + self.pool = self.create_pool(self.lb_id, constants.PROTOCOL_HTTP, + constants.LB_ALGORITHM_ROUND_ROBIN) + bad_pool_id = self.pool.get('pool').get('id') + self.set_lb_status(self.lb_id) + # Create a member on our reference pool + api_member = self.create_member( + self.pool_with_listener_id, '192.0.2.1', 80).get(self.root_tag) + self.set_lb_status(self.lb_id) + # Attempt to delete the member using the wrong pool in the path + member_path = self.MEMBERS_PATH.format( + pool_id=bad_pool_id) + '/' + api_member['id'] + result = self.delete(member_path, status=404).json + ref_msg = 'Member %s not found.' % api_member['id'] + self.assertEqual(ref_msg, result.get('faultstring')) + @mock.patch('octavia.api.drivers.utils.call_provider') def test_delete_with_bad_provider(self, mock_provider): api_member = self.create_member(