From cd9b74679f650e17433f8247649d531c01caf2c9 Mon Sep 17 00:00:00 2001 From: ImMin5 Date: Mon, 28 Apr 2025 21:00:57 +0900 Subject: [PATCH 1/2] feat: enhance recursive action checks with project ID tracking --- .../monitoring/service/event_rule_service.py | 116 ++++++++++-------- 1 file changed, 65 insertions(+), 51 deletions(-) diff --git a/src/spaceone/monitoring/service/event_rule_service.py b/src/spaceone/monitoring/service/event_rule_service.py index a6ce76b..4b74289 100644 --- a/src/spaceone/monitoring/service/event_rule_service.py +++ b/src/spaceone/monitoring/service/event_rule_service.py @@ -1,5 +1,6 @@ import fnmatch import logging +from copy import deepcopy from spaceone.core.service import * @@ -82,6 +83,7 @@ def create(self, params: dict) -> EventRule: project_id = params.get("project_id") domain_id = params["domain_id"] workspace_id = params["workspace_id"] + project_ids = [] identity_mgr: IdentityManager = self.locator.get_manager("IdentityManager") @@ -89,6 +91,7 @@ def create(self, params: dict) -> EventRule: project_info = identity_mgr.get_project(project_id) params["workspace_id"] = project_info.get("workspace_id") workspace_id = params["workspace_id"] + project_ids.append(project_info.get("project_id")) else: identity_mgr.check_workspace(workspace_id, domain_id) params["project_id"] = "*" @@ -98,12 +101,12 @@ def create(self, params: dict) -> EventRule: self._check_actions(params["actions"], domain_id, workspace_id, identity_mgr) self._check_recursive_actions( - identity_mgr, params["actions"], domain_id, workspace_id, [] + identity_mgr, params["actions"], domain_id, workspace_id, project_ids ) params["order"] = ( - self._get_highest_order(resource_group, project_id, domain_id, workspace_id) - + 1 + self._get_highest_order(resource_group, project_id, domain_id, workspace_id) + + 1 ) return self.event_rule_mgr.create_event_rule(params) @@ -138,15 +141,20 @@ def update(self, params: dict) -> EventRule: domain_id = params["domain_id"] workspace_id = params["workspace_id"] user_projects = params.get("user_projects") + project_ids = [] event_rule_vo = self.event_rule_mgr.get_event_rule( event_rule_id, domain_id, workspace_id, user_projects ) + if event_rule_vo.resource_group == "PROJECT": + project_ids.append(event_rule_vo.project_id) + if "conditions" in params: self._check_conditions(params["conditions"]) - if actions := params.get("actions", {}): + actions = params.get("actions") or event_rule_vo.actions + if actions: identity_mgr: IdentityManager = self.locator.get_manager("IdentityManager") self._check_actions( actions, @@ -154,8 +162,9 @@ def update(self, params: dict) -> EventRule: workspace_id, identity_mgr, ) + self._check_recursive_actions( - identity_mgr, actions, domain_id, workspace_id, [] + identity_mgr, actions, domain_id, workspace_id, project_ids ) return self.event_rule_mgr.update_event_rule_by_vo(params, event_rule_vo) @@ -386,26 +395,26 @@ def _check_conditions(conditions: list) -> None: ) if key not in _SUPPORTED_CONDITION_KEYS and not fnmatch.fnmatch( - key, "additional_info.*" + key, "additional_info.*" ): raise ERROR_INVALID_PARAMETER( key="conditions.key", reason=f"Unsupported key. " - f'({" | ".join(_SUPPORTED_CONDITION_KEYS)})', + f'({" | ".join(_SUPPORTED_CONDITION_KEYS)})', ) if operator not in _SUPPORTED_CONDITION_OPERATORS: raise ERROR_INVALID_PARAMETER( key="conditions.operator", reason=f"Unsupported operator. " - f'({" | ".join(_SUPPORTED_CONDITION_OPERATORS)})', + f'({" | ".join(_SUPPORTED_CONDITION_OPERATORS)})', ) def _check_actions( - self, - actions: dict, - domain_id: str, - workspace_id: str, - identity_mgr: IdentityManager, + self, + actions: dict, + domain_id: str, + workspace_id: str, + identity_mgr: IdentityManager, ) -> None: if change_project_id := actions.get("change_project"): identity_mgr.get_project(change_project_id, domain_id) @@ -436,37 +445,42 @@ def _check_actions( ) def _check_recursive_actions( - self, - identity_mgr: IdentityManager, - actions: dict, - domain_id: str, - workspace_id: str, - project_ids: list, - ) -> list: - if change_project_id := actions.get("change_project"): - if change_project_id in project_ids: - _LOGGER.error( - f"[_check_recursive_actions] change_project_id {change_project_id}, project ids: {project_ids} " - ) - raise ERROR_INVALID_PARAMETER( - key="actions.change_project", - reason=f"The {change_project_id} in the action should be different from the project_id in the event rule.", - ) - else: - project_ids.append(change_project_id) - event_rule_vos = self.event_rule_mgr.filter_event_rules( - domain_id=domain_id, - workspace_id=workspace_id, - project_id=change_project_id, - ) - for event_rule_vo in event_rule_vos: - self._check_recursive_actions( - identity_mgr, - event_rule_vo.actions, - domain_id, - workspace_id, - project_ids, - ) + self, + identity_mgr: IdentityManager, + actions: dict, + domain_id: str, + workspace_id: str, + project_ids: list, + ) -> None: + visited_projects = deepcopy(project_ids) + + change_project_id = actions.get("change_project") + if not change_project_id: + return + + if change_project_id in visited_projects: + _LOGGER.error( + f"[_check_recursive_actions] change_project_id {change_project_id}, project ids: {project_ids} " + ) + raise ERROR_INVALID_PARAMETER( + key="actions.change_project", + reason=f"The {change_project_id} in the action should be different from the project_id in the event rule.", + ) + + visited_projects.append(change_project_id) + event_rule_vos = self.event_rule_mgr.filter_event_rules( + domain_id=domain_id, + workspace_id=workspace_id, + project_id=change_project_id, + ) + for event_rule_vo in event_rule_vos: + self._check_recursive_actions( + identity_mgr, + event_rule_vo.actions, + domain_id, + workspace_id, + visited_projects, + ) @staticmethod def _check_order(order: int) -> None: @@ -476,7 +490,7 @@ def _check_order(order: int) -> None: ) def _get_highest_order( - self, resource_group: str, project_id: str, domain_id: str, workspace_id: str + self, resource_group: str, project_id: str, domain_id: str, workspace_id: str ) -> int: query = { "filter": [ @@ -492,12 +506,12 @@ def _get_highest_order( return total_count def _get_all_event_rules( - self, - resource_group: str, - project_id: str, - domain_id: str, - workspace_id: str, - exclude_event_rule_id: str = None, + self, + resource_group: str, + project_id: str, + domain_id: str, + workspace_id: str, + exclude_event_rule_id: str = None, ): query = { "filter": [ From 7f49d4e0d8d49ee5bd7e128bf80f9705854d3e38 Mon Sep 17 00:00:00 2001 From: ImMin5 Date: Tue, 29 Apr 2025 10:33:06 +0900 Subject: [PATCH 2/2] feat: modify var name for clarity --- .../monitoring/service/event_rule_service.py | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/spaceone/monitoring/service/event_rule_service.py b/src/spaceone/monitoring/service/event_rule_service.py index 4b74289..83e174c 100644 --- a/src/spaceone/monitoring/service/event_rule_service.py +++ b/src/spaceone/monitoring/service/event_rule_service.py @@ -105,8 +105,8 @@ def create(self, params: dict) -> EventRule: ) params["order"] = ( - self._get_highest_order(resource_group, project_id, domain_id, workspace_id) - + 1 + self._get_highest_order(resource_group, project_id, domain_id, workspace_id) + + 1 ) return self.event_rule_mgr.create_event_rule(params) @@ -395,26 +395,26 @@ def _check_conditions(conditions: list) -> None: ) if key not in _SUPPORTED_CONDITION_KEYS and not fnmatch.fnmatch( - key, "additional_info.*" + key, "additional_info.*" ): raise ERROR_INVALID_PARAMETER( key="conditions.key", reason=f"Unsupported key. " - f'({" | ".join(_SUPPORTED_CONDITION_KEYS)})', + f'({" | ".join(_SUPPORTED_CONDITION_KEYS)})', ) if operator not in _SUPPORTED_CONDITION_OPERATORS: raise ERROR_INVALID_PARAMETER( key="conditions.operator", reason=f"Unsupported operator. " - f'({" | ".join(_SUPPORTED_CONDITION_OPERATORS)})', + f'({" | ".join(_SUPPORTED_CONDITION_OPERATORS)})', ) def _check_actions( - self, - actions: dict, - domain_id: str, - workspace_id: str, - identity_mgr: IdentityManager, + self, + actions: dict, + domain_id: str, + workspace_id: str, + identity_mgr: IdentityManager, ) -> None: if change_project_id := actions.get("change_project"): identity_mgr.get_project(change_project_id, domain_id) @@ -445,20 +445,20 @@ def _check_actions( ) def _check_recursive_actions( - self, - identity_mgr: IdentityManager, - actions: dict, - domain_id: str, - workspace_id: str, - project_ids: list, + self, + identity_mgr: IdentityManager, + actions: dict, + domain_id: str, + workspace_id: str, + project_ids: list, ) -> None: - visited_projects = deepcopy(project_ids) + visited_project_ids = deepcopy(project_ids) change_project_id = actions.get("change_project") if not change_project_id: return - if change_project_id in visited_projects: + if change_project_id in visited_project_ids: _LOGGER.error( f"[_check_recursive_actions] change_project_id {change_project_id}, project ids: {project_ids} " ) @@ -467,7 +467,7 @@ def _check_recursive_actions( reason=f"The {change_project_id} in the action should be different from the project_id in the event rule.", ) - visited_projects.append(change_project_id) + visited_project_ids.append(change_project_id) event_rule_vos = self.event_rule_mgr.filter_event_rules( domain_id=domain_id, workspace_id=workspace_id, @@ -479,7 +479,7 @@ def _check_recursive_actions( event_rule_vo.actions, domain_id, workspace_id, - visited_projects, + visited_project_ids, ) @staticmethod @@ -490,7 +490,7 @@ def _check_order(order: int) -> None: ) def _get_highest_order( - self, resource_group: str, project_id: str, domain_id: str, workspace_id: str + self, resource_group: str, project_id: str, domain_id: str, workspace_id: str ) -> int: query = { "filter": [ @@ -506,12 +506,12 @@ def _get_highest_order( return total_count def _get_all_event_rules( - self, - resource_group: str, - project_id: str, - domain_id: str, - workspace_id: str, - exclude_event_rule_id: str = None, + self, + resource_group: str, + project_id: str, + domain_id: str, + workspace_id: str, + exclude_event_rule_id: str = None, ): query = { "filter": [