From dedc6d07ac450423bd2d73d88e7ff41303a99742 Mon Sep 17 00:00:00 2001 From: marcus loeper Date: Tue, 4 Mar 2025 14:14:36 +0100 Subject: [PATCH 01/15] Remove raven sentry library --- custom-requirements.txt | 1 - keystone/server/flask/core.py | 24 ------------------------ 2 files changed, 25 deletions(-) diff --git a/custom-requirements.txt b/custom-requirements.txt index 33936bab49..71046688e2 100644 --- a/custom-requirements.txt +++ b/custom-requirements.txt @@ -1,6 +1,5 @@ python-binary-memcached -git+https://github.com/sapcc/raven-python.git@ccloud#egg=raven[flask] git+https://github.com/sapcc/openstack-watcher-middleware.git#egg=watcher-middleware git+https://github.com/funkyHat/py-radius@install_requires#egg=py-radius git+https://github.com/sapcc/keystone-extensions.git@stable/2024.1-m3#egg=keystone-extensions diff --git a/keystone/server/flask/core.py b/keystone/server/flask/core.py index fba5ccc096..16d0d7f393 100644 --- a/keystone/server/flask/core.py +++ b/keystone/server/flask/core.py @@ -25,15 +25,11 @@ from keystone.common import profiler import keystone.conf -from keystone import exception import keystone.server from keystone.server.flask import application from keystone.server.flask.request_processing.middleware import auth_context from keystone.server.flask.request_processing.middleware import url_normalize -# CCloud -import logging -from raven.contrib.flask import Sentry # NOTE(morgan): Middleware Named Tuple with the following values: # * "namespace": namespace for the entry_point @@ -177,26 +173,6 @@ def setup_app_middleware(app): # Apply werkzeug specific middleware app.wsgi_app = proxy_fix.ProxyFix(app.wsgi_app) - # CCloud - if os.environ.get('SENTRY_DSN', None): - processors = ( - 'raven.processors.SanitizePasswordsProcessor', - 'raven.processors.SanitizeKeysProcessor', - 'raven.processors.RemovePostDataProcessor', - ) - sanitize_keys = [ - 'old_password', 'new_password', 'password', 'cred', 'secret', - 'passwd', 'credentials'] - app.config['SENTRY_CONFIG'] = { - 'ignore_exceptions': [exception.NotFound, exception.Unauthorized, - 'INVALID_CREDENTIALS'], - 'processors': processors, - 'sanitize_keys': sanitize_keys, - } - - sentry = Sentry() - sentry.init_app(app, logging=True, level=logging.ERROR) - return app From ebc0c12cb6441f47301481c106c6ad6c303af709 Mon Sep 17 00:00:00 2001 From: marcus loeper Date: Tue, 4 Mar 2025 14:15:26 +0100 Subject: [PATCH 02/15] Add sentry-sdk v1.45.1 library --- keystone/wsgi/sentry_wsgi_middleware.py | 41 +++++++++++++++++++++++++ requirements.txt | 2 ++ 2 files changed, 43 insertions(+) create mode 100644 keystone/wsgi/sentry_wsgi_middleware.py diff --git a/keystone/wsgi/sentry_wsgi_middleware.py b/keystone/wsgi/sentry_wsgi_middleware.py new file mode 100644 index 0000000000..8af1448357 --- /dev/null +++ b/keystone/wsgi/sentry_wsgi_middleware.py @@ -0,0 +1,41 @@ +from keystone.server.wsgi import initialize_public_application + +import sentry_sdk +from sentry_sdk.integrations import wsgi + +import logging +import os +import sys + +LOG = logging.getLogger(__name__) + +handler = logging.StreamHandler(sys.stdout) +LOG.addHandler(handler) + +# get exceptions list as comma separated string from environment +sentry_exclusion_list_string = os.environ.get("SENTRY_EXCLUSIONS_LIST", "") +sentry_exclusion_list = sentry_exclusion_list_string.split(",") +# filtered_exceptions_names = ["Unauthorized", "LDAPInvalidCredentialsError"] + +# check for SENTRY_DSN +# make sure to use the old lagacy DSN fromat because Sentry Instance is version 9.1.2 +sentry_dsn = os.environ.get("SENTRY_DSN", None) +if sentry_dsn is None: + LOG.warning("SENTRY_DSN not found in Environment") + + +def before_send(event, _): + if "exception" in event: + for value in event["exception"]["values"]: + exception_type = value["type"] + if exception_type in sentry_exclusion_list: + LOG.info(f"[-] filtered out: {exception_type}") + return None + LOG.info("[+] send event") + return event + + +sentry_sdk.init(dsn=sentry_dsn, debug=False, before_send=before_send) + +application = initialize_public_application() +application = wsgi.SentryWsgiMiddleware(application) diff --git a/requirements.txt b/requirements.txt index 14d7714a20..d527e4ed70 100644 --- a/requirements.txt +++ b/requirements.txt @@ -43,3 +43,5 @@ pytz>=2013.6 # MIT python-ldap>=3.0.0 # PSF ldappool>=2.3.1 # MPL python-memcached>=1.56 # PSF + +sentry-sdk==1.45.1 # MIT From bcaa65f43ad5de3773e384bc6e81ef1b7e6563a8 Mon Sep 17 00:00:00 2001 From: marcus loeper Date: Tue, 4 Mar 2025 14:54:38 +0100 Subject: [PATCH 03/15] Replace wsgi-script and adjusted the naming --- keystone/wsgi/sentry_wsgi_middleware.py | 5 +++-- setup.cfg | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/keystone/wsgi/sentry_wsgi_middleware.py b/keystone/wsgi/sentry_wsgi_middleware.py index 8af1448357..6b1216bd62 100644 --- a/keystone/wsgi/sentry_wsgi_middleware.py +++ b/keystone/wsgi/sentry_wsgi_middleware.py @@ -15,7 +15,7 @@ # get exceptions list as comma separated string from environment sentry_exclusion_list_string = os.environ.get("SENTRY_EXCLUSIONS_LIST", "") sentry_exclusion_list = sentry_exclusion_list_string.split(",") -# filtered_exceptions_names = ["Unauthorized", "LDAPInvalidCredentialsError"] +# ["Unauthorized", "LDAPInvalidCredentialsError"] # check for SENTRY_DSN # make sure to use the old lagacy DSN fromat because Sentry Instance is version 9.1.2 @@ -38,4 +38,5 @@ def before_send(event, _): sentry_sdk.init(dsn=sentry_dsn, debug=False, before_send=before_send) application = initialize_public_application() -application = wsgi.SentryWsgiMiddleware(application) +sentry_wsgi_public_wrapper = wsgi.SentryWsgiMiddleware(application) +sentry_wsgi_admin_wrapper = sentry_wsgi_public_wrapper diff --git a/setup.cfg b/setup.cfg index bce351e7a8..bd34ed6e7b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -42,8 +42,8 @@ console_scripts = keystone-status = keystone.cmd.status:main wsgi_scripts = - keystone-wsgi-admin = keystone.server.wsgi:initialize_admin_application - keystone-wsgi-public = keystone.server.wsgi:initialize_public_application + keystone-wsgi-admin = keystone.wsgi.sentry_wsgi_middleware:sentry_wsgi_admin_wrapper + keystone-wsgi-public = keystone.wsgi.sentry_wsgi_middleware:sentry_wsgi_public_wrapper keystone.assignment = sql = keystone.assignment.backends.sql:Assignment From 3acc21967766998d04216dc7d5d6b9c4f334317e Mon Sep 17 00:00:00 2001 From: marcus loeper Date: Wed, 5 Mar 2025 15:51:13 +0100 Subject: [PATCH 04/15] Export wrapper into a function --- keystone/wsgi/sentry_wsgi_middleware.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/keystone/wsgi/sentry_wsgi_middleware.py b/keystone/wsgi/sentry_wsgi_middleware.py index 6b1216bd62..0ffe5c79d7 100644 --- a/keystone/wsgi/sentry_wsgi_middleware.py +++ b/keystone/wsgi/sentry_wsgi_middleware.py @@ -37,6 +37,9 @@ def before_send(event, _): sentry_sdk.init(dsn=sentry_dsn, debug=False, before_send=before_send) -application = initialize_public_application() -sentry_wsgi_public_wrapper = wsgi.SentryWsgiMiddleware(application) +def sentry_wsgi_public_wrapper(): + application = initialize_public_application() + sentry_wsgi_public_app = wsgi.SentryWsgiMiddleware(application) + return sentry_wsgi_public_app + sentry_wsgi_admin_wrapper = sentry_wsgi_public_wrapper From 47fa63127f8173f305135e2d773aa1e5a5c5c2da Mon Sep 17 00:00:00 2001 From: marcus loeper Date: Fri, 7 Mar 2025 09:47:05 +0100 Subject: [PATCH 05/15] Reorder Imports and check keys before accessing --- keystone/wsgi/sentry_wsgi_middleware.py | 28 ++++++++++++------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/keystone/wsgi/sentry_wsgi_middleware.py b/keystone/wsgi/sentry_wsgi_middleware.py index 0ffe5c79d7..2cac9b50b6 100644 --- a/keystone/wsgi/sentry_wsgi_middleware.py +++ b/keystone/wsgi/sentry_wsgi_middleware.py @@ -1,16 +1,13 @@ +import logging +import os + from keystone.server.wsgi import initialize_public_application import sentry_sdk from sentry_sdk.integrations import wsgi -import logging -import os -import sys - -LOG = logging.getLogger(__name__) -handler = logging.StreamHandler(sys.stdout) -LOG.addHandler(handler) +logger = logging.getLogger(__name__) # get exceptions list as comma separated string from environment sentry_exclusion_list_string = os.environ.get("SENTRY_EXCLUSIONS_LIST", "") @@ -18,20 +15,21 @@ # ["Unauthorized", "LDAPInvalidCredentialsError"] # check for SENTRY_DSN -# make sure to use the old lagacy DSN fromat because Sentry Instance is version 9.1.2 +# make sure to use the old lagacy DSN fromat sentry_dsn = os.environ.get("SENTRY_DSN", None) if sentry_dsn is None: - LOG.warning("SENTRY_DSN not found in Environment") + logger.warning("SENTRY_DSN not found in Environment") def before_send(event, _): if "exception" in event: - for value in event["exception"]["values"]: - exception_type = value["type"] - if exception_type in sentry_exclusion_list: - LOG.info(f"[-] filtered out: {exception_type}") - return None - LOG.info("[+] send event") + if "values" in event["exception"]: + for value in event["exception"]["values"]: + if "type" in value: + exception_type = value["type"] + if exception_type in sentry_exclusion_list: + logger.info("[-] filtered out: %s", exception_type) + return None return event From 0f5eb85dccf0826be5a9739c9ba8a3ddeb77a078 Mon Sep 17 00:00:00 2001 From: marcus loeper Date: Fri, 7 Mar 2025 11:42:47 +0100 Subject: [PATCH 06/15] Add from sentry suggested uWSGI parameters --- httpd/keystone-uwsgi-admin.ini | 3 +++ httpd/keystone-uwsgi-public.ini | 3 +++ 2 files changed, 6 insertions(+) diff --git a/httpd/keystone-uwsgi-admin.ini b/httpd/keystone-uwsgi-admin.ini index fe7ef7bb4d..849f88fd24 100644 --- a/httpd/keystone-uwsgi-admin.ini +++ b/httpd/keystone-uwsgi-admin.ini @@ -23,3 +23,6 @@ plugins = python # This ensures that file descriptors aren't shared between keystone processes. lazy-apps = true + +# sentry proposes to set these to true +py-call-uwsgi-fork-hooks = true diff --git a/httpd/keystone-uwsgi-public.ini b/httpd/keystone-uwsgi-public.ini index 8decf2023a..bd6ff353f4 100644 --- a/httpd/keystone-uwsgi-public.ini +++ b/httpd/keystone-uwsgi-public.ini @@ -23,3 +23,6 @@ plugins = python # This ensures that file descriptors aren't shared between keystone processes. lazy-apps = true + +# sentry proposes to set these to true +py-call-uwsgi-fork-hooks = true From 06fc3bedd6578c45d5e99192dd642806cd5ada72 Mon Sep 17 00:00:00 2001 From: marcus loeper Date: Fri, 7 Mar 2025 17:53:31 +0100 Subject: [PATCH 07/15] Add exception handling that Keystone does not crash when provided with wrong SENTRY_DSN format --- keystone/wsgi/sentry_wsgi_middleware.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/keystone/wsgi/sentry_wsgi_middleware.py b/keystone/wsgi/sentry_wsgi_middleware.py index 2cac9b50b6..a2e7c0dbab 100644 --- a/keystone/wsgi/sentry_wsgi_middleware.py +++ b/keystone/wsgi/sentry_wsgi_middleware.py @@ -1,6 +1,8 @@ import logging import os +from sentry_sdk.utils import BadDsn + from keystone.server.wsgi import initialize_public_application import sentry_sdk @@ -28,12 +30,16 @@ def before_send(event, _): if "type" in value: exception_type = value["type"] if exception_type in sentry_exclusion_list: - logger.info("[-] filtered out: %s", exception_type) + logger.info("[+] filtered out: %s", exception_type) return None return event - -sentry_sdk.init(dsn=sentry_dsn, debug=False, before_send=before_send) +try: + sentry_sdk.init(dsn=sentry_dsn, debug=False, before_send=before_send) +except BadDsn as e: + logger.error("Bad SENTRY_DSN format, %s: expected https://:@/:id", e) +except Exception as e: + logger.error("Exception while initializing sentry sdk, %s", e) def sentry_wsgi_public_wrapper(): application = initialize_public_application() From ac38bf33a5ba3698bfbf08bd280e259b15069e19 Mon Sep 17 00:00:00 2001 From: Marcus Loeper Date: Fri, 24 Oct 2025 16:00:16 +0200 Subject: [PATCH 08/15] Implement Sentry filtering engine and rule loader with comprehensive tests - Added SentryFilterEngine for filtering Sentry events based on configurable rules. - Introduced RuleValidationError for handling rule validation errors. - Created load_rules_from_file function to load and validate rules from YAML configuration. - Developed unit tests for SentryFilterEngine and rule loader to ensure functionality and validation. - Updated Sentry WSGI middleware to integrate filtering engine and handle environment configurations. - Added example YAML configuration for Sentry filtering rules. - Updated requirements to use the latest version of sentry-sdk. --- keystone/common/sentry_filter_engine.py | 195 ++++++++++ keystone/common/sentry_rule_loader.py | 200 +++++++++++ .../unit/common/test_sentry_filter_engine.py | 176 +++++++++ .../unit/common/test_sentry_rule_loader.py | 336 ++++++++++++++++++ keystone/wsgi/sentry_wsgi_middleware.py | 95 +++-- requirements.txt | 2 +- sentry_filters_example.yaml | 86 +++++ 7 files changed, 1059 insertions(+), 31 deletions(-) create mode 100644 keystone/common/sentry_filter_engine.py create mode 100644 keystone/common/sentry_rule_loader.py create mode 100644 keystone/tests/unit/common/test_sentry_filter_engine.py create mode 100644 keystone/tests/unit/common/test_sentry_rule_loader.py create mode 100644 sentry_filters_example.yaml diff --git a/keystone/common/sentry_filter_engine.py b/keystone/common/sentry_filter_engine.py new file mode 100644 index 0000000000..e3424279b3 --- /dev/null +++ b/keystone/common/sentry_filter_engine.py @@ -0,0 +1,195 @@ +# 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. + +import re +import time +import logging +from collections import defaultdict, deque +from typing import Dict, List, Any, Optional + +LOG = logging.getLogger(__name__) + + +class SentryFilterEngine: + """Engine for filtering Sentry events based on configurable rules.""" + + def __init__(self, rules: List[Dict[str, Any]], debug_logging: bool = False): + """Initialize the filter engine with rules. + + Args: + rules: List of validated rule dictionaries + debug_logging: Enable detailed debug logging for filtering decisions + """ + self.rules = rules + self.debug_logging = debug_logging + self._rate_limit_tracker = defaultdict(deque) + self._compiled_regexes = {} + + LOG.info("Initialized Sentry filter engine with %d rules", len(rules)) + self.debug_log("Debug logging enabled for Sentry filtering") + + def should_filter_event(self, event: Dict[str, Any]) -> bool: + """Determine if an event should be filtered out. + + Args: + event: Sentry event dictionary + + Returns: + True if event should be filtered out (not sent to Sentry) + False if event should be sent to Sentry + """ + if not self.rules: + self.debug_log("No rules configured, allowing event") + return False + + for rule in self.rules: + if self._rule_matches(rule, event): + rule_name = rule.get('name', 'unnamed') + self.debug_log("Rule '%s' matched, filtering event", rule_name) + LOG.info("Filtered Sentry event by rule: %s", rule_name) + return True + + self.debug_log("No rules matched, allowing event") + return False + + def _rule_matches(self, rule: Dict[str, Any], event: Dict[str, Any]) -> bool: + """Check if all conditions in a rule match the event.""" + rule_name = rule.get('name', 'unnamed') + + conditions = [ + ('exception_type', lambda: self._match_exception_type(rule['exception_type'], event)), + ('message_pattern', lambda: self._match_message_pattern(rule['message_pattern'], event)), + ('message_contains', lambda: self._match_message_contains(rule['message_contains'], event)), + ('rate_limit', lambda: self._check_rate_limit(rule, event)), + ] + + for condition_key, checker in conditions: + if condition_key in rule: + if checker(): + self.debug_log("Rule '%s': %s condition matched", rule_name, condition_key) + else: + self.debug_log("Rule '%s': %s condition did not match", rule_name, condition_key) + return False + + return True + + def _match_exception_type(self, rule_type: str, event: Dict[str, Any]) -> bool: + """Check if event exception type matches rule. + + Args: + rule_type: Exception type from rule + event: Sentry event dictionary + + Returns: + True if exception type matches + """ + exception_values = event.get('exception', {}).get('values', []) + for exc_value in exception_values: + if exc_value.get('type') == rule_type: + return True + return False + + def _match_message_pattern(self, pattern: str, event: Dict[str, Any]) -> bool: + """Check if event exception message matches regex pattern. + + Args: + pattern: Regex pattern string + event: Sentry event dictionary + + Returns: + True if message matches pattern + """ + # Cache compiled regex for performance + if pattern not in self._compiled_regexes: + self._compiled_regexes[pattern] = re.compile(pattern) + + regex = self._compiled_regexes[pattern] + exception_values = event.get('exception', {}).get('values', []) + + for exc_value in exception_values: + message = exc_value.get('value', '') + if regex.search(message): + return True + return False + + def _match_message_contains(self, search_string: str, event: Dict[str, Any]) -> bool: + """Check if event exception message contains string. + + Args: + search_string: String to search for + event: Sentry event dictionary + + Returns: + True if message contains string + """ + exception_values = event.get('exception', {}).get('values', []) + for exc_value in exception_values: + message = exc_value.get('value', '') + if search_string in message: + return True + return False + + def _check_rate_limit(self, rule: Dict[str, Any], event: Dict[str, Any]) -> bool: + """Check if rule should apply based on rate limiting. + + Args: + rule: Rule dictionary with rate limiting parameters + event: Sentry event dictionary + + Returns: + True if rule should apply (event should be filtered) + False if not at rate limit yet (event should not be filtered) + """ + rule_name = rule.get('name', 'unnamed') + current_time = time.time() + + rate_limit = rule['rate_limit'] + occurrences = self._rate_limit_tracker[rule_name] + cutoff_time = current_time - rate_limit['time_window'] + + # Clean old entries + removed_count = 0 + while occurrences and occurrences[0] < cutoff_time: + occurrences.popleft() + removed_count += 1 + + if self.debug_logging and removed_count > 0: + LOG.debug("Rule '%s': cleaned %d old occurrences", rule_name, removed_count) + + # Check if we've hit the limit + current_count = len(occurrences) + max_occurrences = rate_limit['max_occurrences'] + + if current_count >= max_occurrences: + if self.debug_logging: + LOG.debug("Rule '%s': rate limit reached (%d >= %d)", + rule_name, current_count, max_occurrences) + return True # Rule applies - filter this event + + # Add current occurrence + occurrences.append(current_time) + + if self.debug_logging: + LOG.debug("Rule '%s': added occurrence (%d/%d in %ds window)", + rule_name, current_count + 1, max_occurrences, rate_limit['time_window']) + + return False # Not at limit yet - don't filter + + + def debug_log(self, *args: Any) -> None: + """Log a debug message if debug logging is enabled. + + Args: + message: Message to log + """ + if self.debug_logging: + LOG.debug(*args) diff --git a/keystone/common/sentry_rule_loader.py b/keystone/common/sentry_rule_loader.py new file mode 100644 index 0000000000..73cc9ef5b9 --- /dev/null +++ b/keystone/common/sentry_rule_loader.py @@ -0,0 +1,200 @@ +# 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. + +import os +import yaml +import logging +from typing import Dict, List, Any + +LOG = logging.getLogger(__name__) + + +class RuleValidationError(Exception): + """Raised when a rule fails validation.""" + pass + + +def load_rules_from_file(config_file: str) -> List[Dict[str, Any]]: + """Load and validate Sentry filtering rules from YAML file. + + Args: + config_file: Path to the YAML configuration file + + Returns: + List of validated rule dictionaries + + Raises: + FileNotFoundError: If config file doesn't exist + yaml.YAMLError: If YAML parsing fails + RuleValidationError: If rule validation fails + """ + LOG.info("Loading Sentry filter rules from: %s", config_file) + + if not os.path.exists(config_file): + # return empty list if config file does not exist + LOG.warning("Sentry filter config file not found: %s", config_file) + return [] + + try: + with open(config_file, 'r') as f: + config_data = yaml.safe_load(f) + except yaml.YAMLError as e: + raise yaml.YAMLError(f"Failed to parse YAML config file {config_file}: {e}") + + if not isinstance(config_data, dict): + raise RuleValidationError("Config file must contain a YAML dictionary") + + if 'rules' not in config_data: + raise RuleValidationError("Config file must contain a 'rules' section") + + rules = config_data['rules'] + if not isinstance(rules, list): + raise RuleValidationError("'rules' section must be a list") + + validated_rules = [] + for i, rule in enumerate(rules): + try: + validated_rule = _validate_rule(rule, i) + validated_rules.append(validated_rule) + except RuleValidationError as e: + LOG.error("Rule validation failed for rule %d: %s", i, e) + raise RuleValidationError(f"Rule {i} validation failed: {e}") + + LOG.info("Successfully loaded %d Sentry filter rules", len(validated_rules)) + return validated_rules + + +def _validate_rule(rule: Dict[str, Any], rule_index: int) -> Dict[str, Any]: + """Validate a single filtering rule. + + Args: + rule: Rule dictionary to validate + rule_index: Index of rule for error reporting + + Returns: + Validated rule dictionary + + Raises: + RuleValidationError: If rule validation fails + """ + if not isinstance(rule, dict): + raise RuleValidationError(f"Rule must be a dictionary, got {type(rule)}") + + # Rule name is required for logging and rate limiting identification + if 'name' not in rule: + raise RuleValidationError("Rule must have a 'name' field") + + if not isinstance(rule['name'], str) or not rule['name'].strip(): + raise RuleValidationError("Rule 'name' must be a non-empty string") + + # At least one filtering condition must be specified + has_condition = any(field in rule for field in [ + 'exception_type', 'message_pattern', 'message_contains' + ]) + + if not has_condition: + raise RuleValidationError( + "Rule must specify at least one condition: " + "'exception_type', 'message_pattern', or 'message_contains'" + ) + + # Validate exception_type + if 'exception_type' in rule: + if not isinstance(rule['exception_type'], str): + raise RuleValidationError("'exception_type' must be a string") + + # Validate message_pattern (regex) + if 'message_pattern' in rule: + if not isinstance(rule['message_pattern'], str): + raise RuleValidationError("'message_pattern' must be a string") + + # Test regex compilation + try: + import re + re.compile(rule['message_pattern']) + except re.error as e: + raise RuleValidationError(f"Invalid regex in 'message_pattern': {e}") + + # Validate message_contains + if 'message_contains' in rule: + if not isinstance(rule['message_contains'], str): + raise RuleValidationError("'message_contains' must be a string") + + # Validate rate limiting parameters + if 'rate_limit' in rule: + rate_limit = rule['rate_limit'] + + if not isinstance(rate_limit, dict): + raise RuleValidationError("'rate_limit' must be a dictionary") + + if 'max_occurrences' not in rate_limit or 'time_window' not in rate_limit: + raise RuleValidationError( + "Rate limiting requires both 'max_occurrences' and 'time_window' in 'rate_limit' section" + ) + + if not isinstance(rate_limit['max_occurrences'], int) or rate_limit['max_occurrences'] <= 0: + raise RuleValidationError("'rate_limit.max_occurrences' must be a positive integer") + + if not isinstance(rate_limit['time_window'], (int, float)) or rate_limit['time_window'] <= 0: + raise RuleValidationError("'rate_limit.time_window' must be a positive number") + + return rule + + +def create_example_config_file(output_file: str) -> None: + """Create an example configuration file with common filtering rules. + + Args: + output_file: Path where to write the example config + """ + example_config = { + 'rules': [ + { + 'name': 'exclude_unauthorized', + 'exception_type': 'Unauthorized' + }, + { + 'name': 'exclude_ldap_credentials', + 'exception_type': 'LDAPInvalidCredentialsError' + }, + { + 'name': 'rate_limit_ldap_connection', + 'exception_type': 'LDAPServerConnectionError', + "rate_limit": { + "max_occurrences": 5, + "time_window": 300 + } + }, + { + 'name': 'filter_timeout_messages', + 'message_contains': 'timeout' + }, + { + 'name': 'filter_connection_refused', + 'message_pattern': 'Connection.*refused.*port\s+\d+' + }, + { + 'name': 'complex_database_rule', + 'exception_type': 'DatabaseError', + 'message_contains': 'deadlock', + 'rate_limit': { + 'max_occurrences': 3, + 'time_window': 180 + } + } + ] + } + + with open(output_file, 'w') as f: + yaml.dump(example_config, f, default_flow_style=False, indent=2) + + LOG.info("Created example Sentry filter config at: %s", output_file) diff --git a/keystone/tests/unit/common/test_sentry_filter_engine.py b/keystone/tests/unit/common/test_sentry_filter_engine.py new file mode 100644 index 0000000000..e109ec7565 --- /dev/null +++ b/keystone/tests/unit/common/test_sentry_filter_engine.py @@ -0,0 +1,176 @@ +# 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. + +import time +from unittest import mock + +from keystone.common.sentry_filter_engine import SentryFilterEngine +from keystone.tests import unit + + +class SentryFilterEngineTestCase(unit.BaseTestCase): + """Test suite for Sentry filter engine functionality.""" + + def setUp(self): + """Set up test fixtures.""" + super(SentryFilterEngineTestCase, self).setUp() + + def test_no_rules_allows_all_events(self): + """Test that engine with no rules allows all events.""" + engine = SentryFilterEngine([]) + event = {'exception': {'values': [{'type': 'TestError', 'value': 'test message'}]}} + + result = engine.should_filter_event(event) + self.assertFalse(result) + + def test_exception_type_matching(self): + """Test basic exception type matching.""" + rules = [{'name': 'test_rule', 'exception_type': 'TestError'}] + engine = SentryFilterEngine(rules) + + # Matching event should be filtered + matching_event = {'exception': {'values': [{'type': 'TestError', 'value': 'test'}]}} + result = engine.should_filter_event(matching_event) + self.assertTrue(result) + + # Non-matching event should not be filtered + non_matching_event = {'exception': {'values': [{'type': 'OtherError', 'value': 'test'}]}} + result = engine.should_filter_event(non_matching_event) + self.assertFalse(result) + + def test_message_contains_matching(self): + """Test basic message contains matching.""" + rules = [{'name': 'test_rule', 'message_contains': 'timeout'}] + engine = SentryFilterEngine(rules) + + # Matching event should be filtered + matching_event = {'exception': {'values': [{'type': 'Error', 'value': 'connection timeout error'}]}} + result = engine.should_filter_event(matching_event) + self.assertTrue(result) + + # Non-matching event should not be filtered + non_matching_event = {'exception': {'values': [{'type': 'Error', 'value': 'other error'}]}} + result = engine.should_filter_event(non_matching_event) + self.assertFalse(result) + + def test_message_pattern_matching(self): + """Test basic regex pattern matching.""" + rules = [{'name': 'test_rule', 'message_pattern': 'error.*\\d+'}] + engine = SentryFilterEngine(rules) + + # Matching event should be filtered + matching_event = {'exception': {'values': [{'type': 'Error', 'value': 'error code 500'}]}} + result = engine.should_filter_event(matching_event) + self.assertTrue(result) + + # Non-matching event should not be filtered + non_matching_event = {'exception': {'values': [{'type': 'Error', 'value': 'error without number'}]}} + result = engine.should_filter_event(non_matching_event) + self.assertFalse(result) + + def test_rate_limiting_basic_functionality(self): + """Test basic rate limiting functionality.""" + rules = [{ + 'name': 'rate_limited_rule', + 'exception_type': 'TestError', + 'rate_limit': {'max_occurrences': 2, 'time_window': 60} + }] + engine = SentryFilterEngine(rules) + event = {'exception': {'values': [{'type': 'TestError', 'value': 'test'}]}} + + # First occurrence should not be filtered (not at limit yet) + result = engine.should_filter_event(event) + self.assertFalse(result) + + # Second occurrence should not be filtered (not at limit yet) + result = engine.should_filter_event(event) + self.assertFalse(result) + + # Third occurrence should be filtered (at limit) + result = engine.should_filter_event(event) + self.assertTrue(result) + + @mock.patch('time.time') + def test_rate_limiting_time_window(self, mock_time): + """Test that rate limiting respects time windows.""" + rules = [{ + 'name': 'rate_limited_rule', + 'exception_type': 'TestError', + 'rate_limit': {'max_occurrences': 1, 'time_window': 10} + }] + engine = SentryFilterEngine(rules) + event = {'exception': {'values': [{'type': 'TestError', 'value': 'test'}]}} + + # First occurrence at time 0 + mock_time.return_value = 0 + result = engine.should_filter_event(event) + self.assertFalse(result) + + # Second occurrence at time 5 (within window) should be filtered + mock_time.return_value = 5 + result = engine.should_filter_event(event) + self.assertTrue(result) + + # Third occurrence at time 15 (outside window) should not be filtered + mock_time.return_value = 15 + result = engine.should_filter_event(event) + self.assertFalse(result) + + def test_multiple_conditions_must_all_match(self): + """Test that rules with multiple conditions require all to match.""" + rules = [{ + 'name': 'multi_condition_rule', + 'exception_type': 'TestError', + 'message_contains': 'timeout' + }] + engine = SentryFilterEngine(rules) + + # Event matching both conditions should be filtered + matching_event = {'exception': {'values': [{'type': 'TestError', 'value': 'timeout error'}]}} + result = engine.should_filter_event(matching_event) + self.assertTrue(result) + + # Event matching only one condition should not be filtered + partial_match_event = {'exception': {'values': [{'type': 'TestError', 'value': 'other error'}]}} + result = engine.should_filter_event(partial_match_event) + self.assertFalse(result) + + def test_get_statistics(self): + """Test that statistics can be retrieved.""" + rules = [{'name': 'test_rule', 'exception_type': 'TestError'}] + engine = SentryFilterEngine(rules) + + stats = engine.get_statistics() + + # Basic structure should be present + self.assertIn('total_rules', stats) + self.assertIn('rate_limited_rules', stats) + self.assertIn('compiled_regexes', stats) + self.assertIn('rate_limit_stats', stats) + + # Should have correct rule count + self.assertEqual(stats['total_rules'], 1) + + def test_empty_event_handling(self): + """Test handling of malformed or empty events.""" + rules = [{'name': 'test_rule', 'exception_type': 'TestError'}] + engine = SentryFilterEngine(rules) + + # Empty event should not be filtered + empty_event = {} + result = engine.should_filter_event(empty_event) + self.assertFalse(result) + + # Event without exception should not be filtered + no_exception_event = {'other_field': 'value'} + result = engine.should_filter_event(no_exception_event) + self.assertFalse(result) diff --git a/keystone/tests/unit/common/test_sentry_rule_loader.py b/keystone/tests/unit/common/test_sentry_rule_loader.py new file mode 100644 index 0000000000..696da0d067 --- /dev/null +++ b/keystone/tests/unit/common/test_sentry_rule_loader.py @@ -0,0 +1,336 @@ +# 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. + +import os +import tempfile +import yaml + +from keystone.common.sentry_rule_loader import ( + load_rules_from_file, + RuleValidationError +) +from keystone.tests import unit + + +class SentryRuleLoaderTestCase(unit.BaseTestCase): + """Test suite for Sentry rule loader functionality.""" + + def setUp(self): + """Set up test fixtures.""" + super(SentryRuleLoaderTestCase, self).setUp() + self.temp_files = [] + + def tearDown(self): + """Clean up temporary files.""" + for temp_file in self.temp_files: + if os.path.exists(temp_file): + os.unlink(temp_file) + super(SentryRuleLoaderTestCase, self).tearDown() + + def _create_temp_config(self, config_data): + """Create a temporary configuration file with given data.""" + temp_file = tempfile.NamedTemporaryFile(mode='w', suffix='.yaml', delete=False) + yaml.dump(config_data, temp_file, default_flow_style=False) + temp_file.close() + self.temp_files.append(temp_file.name) + return temp_file.name + + def test_load_valid_config(self): + """Test that valid configuration can be loaded without errors.""" + config_data = { + 'rules': [ + {'name': 'test_rule', 'exception_type': 'TestError'} + ] + } + config_file = self._create_temp_config(config_data) + + # Should not raise any exceptions + rules = load_rules_from_file(config_file) + self.assertEqual(len(rules), 1) + self.assertEqual(rules[0]['name'], 'test_rule') + + def test_empty_rules_list(self): + """Test configuration with empty rules list.""" + config_data = {'rules': []} + config_file = self._create_temp_config(config_data) + + rules = load_rules_from_file(config_file) + self.assertEqual(len(rules), 0) + + def test_missing_rules_section(self): + """Test configuration without 'rules' section.""" + config_data = {'other_section': []} + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_nonexistent_file(self): + """Test loading from non-existent file.""" + config_file = '/nonexistent/path/config.yaml' + rules = load_rules_from_file(config_file) + self.assertEqual(len(rules), 0) + + def test_invalid_yaml_format(self): + """Test handling of malformed YAML.""" + temp_file = tempfile.NamedTemporaryFile(mode='w', suffix='.yaml', delete=False) + temp_file.write("invalid: yaml: content: [unclosed") + temp_file.close() + self.temp_files.append(temp_file.name) + + self.assertRaises(yaml.YAMLError, load_rules_from_file, temp_file.name) + + def test_rule_not_dictionary(self): + """Test validation fails when rule is not a dictionary.""" + config_data = { + 'rules': ["not_a_dict"] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rule_missing_name(self): + """Test validation fails when rule has no 'name' field.""" + config_data = { + 'rules': [{'exception_type': 'TestError'}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rule_name_not_string(self): + """Test validation fails when rule name is not a string.""" + config_data = { + 'rules': [{'name': 123, 'exception_type': 'TestError'}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rule_name_empty_string(self): + """Test validation fails when rule name is empty.""" + config_data = { + 'rules': [{'name': '', 'exception_type': 'TestError'}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rule_name_whitespace_only(self): + """Test validation fails when rule name is only whitespace.""" + config_data = { + 'rules': [{'name': ' ', 'exception_type': 'TestError'}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rule_no_conditions(self): + """Test validation fails when rule has no filtering conditions.""" + config_data = { + 'rules': [{'name': 'test_rule'}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_exception_type_not_string(self): + """Test validation fails when exception_type is not a string.""" + config_data = { + 'rules': [{'name': 'test_rule', 'exception_type': 123}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_message_pattern_not_string(self): + """Test validation fails when message_pattern is not a string.""" + config_data = { + 'rules': [{'name': 'test_rule', 'message_pattern': 123}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_message_pattern_invalid_regex(self): + """Test validation fails when message_pattern is invalid regex.""" + config_data = { + 'rules': [{'name': 'test_rule', 'message_pattern': '[unclosed'}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_message_contains_not_string(self): + """Test validation fails when message_contains is not a string.""" + config_data = { + 'rules': [{'name': 'test_rule', 'message_contains': 123}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rate_limit_not_dictionary(self): + """Test validation fails when rate_limit is not a dictionary.""" + config_data = { + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', 'rate_limit': 'not_dict'}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rate_limit_missing_max_occurrences(self): + """Test validation fails when rate_limit missing max_occurrences.""" + config_data = { + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', 'rate_limit': {'time_window': 300}}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rate_limit_missing_time_window(self): + """Test validation fails when rate_limit missing time_window.""" + config_data = { + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', 'rate_limit': {'max_occurrences': 5}}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rate_limit_max_occurrences_not_int(self): + """Test validation fails when max_occurrences is not an integer.""" + config_data = { + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': {'max_occurrences': 'five', 'time_window': 300}}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rate_limit_max_occurrences_zero(self): + """Test validation fails when max_occurrences is zero.""" + config_data = { + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': {'max_occurrences': 0, 'time_window': 300}}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rate_limit_max_occurrences_negative(self): + """Test validation fails when max_occurrences is negative.""" + config_data = { + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': {'max_occurrences': -1, 'time_window': 300}}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rate_limit_time_window_not_number(self): + """Test validation fails when time_window is not a number.""" + config_data = { + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': {'max_occurrences': 5, 'time_window': 'five_minutes'}}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rate_limit_time_window_zero(self): + """Test validation fails when time_window is zero.""" + config_data = { + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': {'max_occurrences': 5, 'time_window': 0}}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rate_limit_time_window_negative(self): + """Test validation fails when time_window is negative.""" + config_data = { + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': {'max_occurrences': 5, 'time_window': -300}}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + def test_rate_limit_time_window_float(self): + """Test validation succeeds when time_window is a float.""" + config_data = { + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': {'max_occurrences': 5, 'time_window': 300.5}}] + } + config_file = self._create_temp_config(config_data) + + # Should not raise any exceptions + rules = load_rules_from_file(config_file) + self.assertEqual(len(rules), 1) + + def test_valid_message_pattern(self): + """Test validation succeeds with valid regex pattern.""" + config_data = { + 'rules': [{'name': 'test_rule', 'message_pattern': 'Connection.*refused.*port\s+\d+'}] + } + config_file = self._create_temp_config(config_data) + + # Should not raise any exceptions + rules = load_rules_from_file(config_file) + self.assertEqual(len(rules), 1) + + def test_valid_message_contains(self): + """Test validation succeeds with message_contains.""" + config_data = { + 'rules': [{'name': 'test_rule', 'message_contains': 'timeout'}] + } + config_file = self._create_temp_config(config_data) + + # Should not raise any exceptions + rules = load_rules_from_file(config_file) + self.assertEqual(len(rules), 1) + + def test_complex_valid_rule(self): + """Test validation succeeds with complex rule having multiple conditions.""" + config_data = { + 'rules': [{ + 'name': 'complex_rule', + 'exception_type': 'DatabaseError', + 'message_contains': 'deadlock', + 'rate_limit': { + 'max_occurrences': 3, + 'time_window': 180 + } + }] + } + config_file = self._create_temp_config(config_data) + + # Should not raise any exceptions + rules = load_rules_from_file(config_file) + self.assertEqual(len(rules), 1) + self.assertEqual(rules[0]['name'], 'complex_rule') + + def test_config_not_dictionary(self): + """Test validation fails when config root is not a dictionary.""" + temp_file = tempfile.NamedTemporaryFile(mode='w', suffix='.yaml', delete=False) + temp_file.write("- not_a_dict") + temp_file.close() + self.temp_files.append(temp_file.name) + + self.assertRaises(RuleValidationError, load_rules_from_file, temp_file.name) + + def test_rules_not_list(self): + """Test validation fails when 'rules' is not a list.""" + config_data = {'rules': 'not_a_list'} + config_file = self._create_temp_config(config_data) + + self.assertRaises(RuleValidationError, load_rules_from_file, config_file) diff --git a/keystone/wsgi/sentry_wsgi_middleware.py b/keystone/wsgi/sentry_wsgi_middleware.py index a2e7c0dbab..7311eb12a8 100644 --- a/keystone/wsgi/sentry_wsgi_middleware.py +++ b/keystone/wsgi/sentry_wsgi_middleware.py @@ -2,48 +2,83 @@ import os from sentry_sdk.utils import BadDsn - -from keystone.server.wsgi import initialize_public_application - import sentry_sdk from sentry_sdk.integrations import wsgi +from keystone.server.wsgi import initialize_public_application +from keystone.common.sentry_filter_engine import SentryFilterEngine +from keystone.common.sentry_rule_loader import load_rules_from_file, RuleValidationError logger = logging.getLogger(__name__) -# get exceptions list as comma separated string from environment -sentry_exclusion_list_string = os.environ.get("SENTRY_EXCLUSIONS_LIST", "") -sentry_exclusion_list = sentry_exclusion_list_string.split(",") -# ["Unauthorized", "LDAPInvalidCredentialsError"] - -# check for SENTRY_DSN -# make sure to use the old lagacy DSN fromat -sentry_dsn = os.environ.get("SENTRY_DSN", None) -if sentry_dsn is None: - logger.warning("SENTRY_DSN not found in Environment") - - -def before_send(event, _): - if "exception" in event: - if "values" in event["exception"]: - for value in event["exception"]["values"]: - if "type" in value: - exception_type = value["type"] - if exception_type in sentry_exclusion_list: - logger.info("[+] filtered out: %s", exception_type) - return None +filter_engine = None + +# Environment variable configuration +# SENTRY_FILTER_CONFIG_FILE: Path to YAML file with filtering rules +# SENTRY_DEBUG_FILTERING: Enable debug logging for filtering (true/false) +# SENTRY_DSN: Sentry DSN for error reporting + +def _initialize_sentry_filtering(): + """Initialize the Sentry filtering engine using environment variables.""" + global filter_engine + + sentry_filter_config = os.environ.get("SENTRY_FILTER_CONFIG_FILE") + debug_filtering_str = os.environ.get("SENTRY_DEBUG_FILTERING", "false").lower() + debug_filtering = debug_filtering_str in ("true", "1", "yes", "on") + + if not sentry_filter_config: + logger.info("SENTRY_FILTER_CONFIG_FILE not set, Sentry filtering disabled") + return + + if not os.path.exists(sentry_filter_config): + logger.warning("Sentry filter config file not found at %s, filtering disabled", sentry_filter_config) + return + + try: + rules = load_rules_from_file(sentry_filter_config) + filter_engine = SentryFilterEngine(rules, debug_logging=debug_filtering) + logger.info("Loaded %d Sentry filter rules from %s", len(rules), sentry_filter_config) + + except RuleValidationError as e: + logger.error("Failed to load Sentry filter config: %s", e) + logger.warning("Sentry filtering disabled due to configuration error") + except Exception as e: + logger.error("Unexpected error loading Sentry filter config: %s", e) + logger.warning("Sentry filtering disabled due to unexpected error") + + +def before_send(event, hint): + """Sentry before_send hook to filter events based on configured rules.""" + if filter_engine and filter_engine.should_filter_event(event): + return None return event -try: - sentry_sdk.init(dsn=sentry_dsn, debug=False, before_send=before_send) -except BadDsn as e: - logger.error("Bad SENTRY_DSN format, %s: expected https://:@/:id", e) -except Exception as e: - logger.error("Exception while initializing sentry sdk, %s", e) + +def _initialize_sentry_sdk(): + """Initialize the Sentry SDK with configuration from environment variables.""" + sentry_dsn = os.environ.get("SENTRY_DSN") + if not sentry_dsn: + logger.warning("SENTRY_DSN not found in environment, Sentry disabled") + return + + try: + sentry_sdk.init(dsn=sentry_dsn, debug=True, before_send=before_send) + logger.info("Sentry SDK initialized successfully") + except BadDsn as e: + logger.error("Bad SENTRY_DSN format: %s", e) + except Exception as e: + logger.error("Exception while initializing Sentry SDK: %s", e) + def sentry_wsgi_public_wrapper(): + """Create and return the WSGI application wrapped with Sentry middleware.""" + + _initialize_sentry_filtering() + _initialize_sentry_sdk() + application = initialize_public_application() sentry_wsgi_public_app = wsgi.SentryWsgiMiddleware(application) return sentry_wsgi_public_app + sentry_wsgi_admin_wrapper = sentry_wsgi_public_wrapper diff --git a/requirements.txt b/requirements.txt index d527e4ed70..bad7b3c0fb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -44,4 +44,4 @@ python-ldap>=3.0.0 # PSF ldappool>=2.3.1 # MPL python-memcached>=1.56 # PSF -sentry-sdk==1.45.1 # MIT +sentry-sdk==2.27.0 # MIT diff --git a/sentry_filters_example.yaml b/sentry_filters_example.yaml new file mode 100644 index 0000000000..6539a344cd --- /dev/null +++ b/sentry_filters_example.yaml @@ -0,0 +1,86 @@ +# Example Sentry Filtering Rules Configuration +# This file demonstrates various filtering rules for Keystone Sentry integration +# +# Rule Types: +# - exception_type: Filter by exception class name +# - message_pattern: Filter by regex pattern in exception message +# - message_contains: Filter by simple string in exception message +# - max_occurrences + time_window: Rate limiting (both required together) +# +# Environment variable to set path: SENTRY_FILTER_CONFIG=/path/to/this/file.yaml + +rules: + # Simple exclusion rules - always filter these exceptions + - name: "exclude_unauthorized_errors" + exception_type: "Unauthorized" + + - name: "exclude_ldap_invalid_credentials" + exception_type: "LDAPInvalidCredentialsError" + + - name: "exclude_token_not_found" + exception_type: "TokenNotFound" + + # Rate limited rules - allow some occurrences, then filter + - name: "rate_limit_ldap_connection_errors" + exception_type: "LDAPServerConnectionError" + rate_limit: + max_occurrences: 5 + time_window: 300 # 5 minutes + + - name: "rate_limit_database_connection_errors" + exception_type: "DatabaseError" + message_contains: "connection" + rate_limit: + max_occurrences: 10 + time_window: 600 # 10 minutes + + # Message-based filtering + - name: "filter_timeout_messages" + message_contains: "timeout" + + - name: "filter_connection_refused_errors" + message_pattern: "Connection.*refused.*port\\s+\\d+" + + - name: "filter_ssl_certificate_errors" + message_pattern: "(?i)ssl.*certificate.*(?:expired|invalid|verify failed)" + + # Complex rules combining multiple conditions + - name: "complex_database_deadlock_rule" + exception_type: "DatabaseError" + message_contains: "deadlock" + rate_limit: + max_occurrences: 3 + time_window: 180 # 3 minutes + + - name: "rate_limit_validation_errors" + exception_type: "ValidationError" + message_pattern: ".*length.*exceeded.*" + rate_limit: + max_occurrences: 20 + time_window: 300 # 5 minutes + + # Health check and monitoring related + - name: "filter_health_check_errors" + message_contains: "/health" + + - name: "filter_metrics_endpoint_errors" + message_pattern: ".*/metrics.*" + + # Development and testing related (might want to disable in production) + - name: "filter_test_related_errors" + message_contains: "test_" + rate_limit: + max_occurrences: 1 + time_window: 3600 # 1 hour - only log once per hour in case tests run in prod + +# Configuration guidelines: +# +# 1. Rule names should be descriptive and unique +# 2. Use rate limiting for intermittent issues you want to be aware of but not spammed by +# 3. Use simple exclusion for known issues that are handled elsewhere or not actionable +# 4. Use message_pattern for complex regex matching, message_contains for simple string matching +# 5. Combine multiple conditions (exception_type + message_contains) for precise filtering +# 6. Test regex patterns before deployment - invalid regex will prevent rule loading +# 7. Consider different time_window values based on issue frequency: +# - Short windows (60-300s) for high-frequency issues +# - Long windows (600-3600s) for occasional issues you want to limit From e6451a289a1a59906f94f0061638573aa9e45f15 Mon Sep 17 00:00:00 2001 From: Marcus Loeper Date: Fri, 24 Oct 2025 16:33:20 +0200 Subject: [PATCH 09/15] Enhance Sentry SDK initialization with event scrubber and denylist for sensitive data --- keystone/wsgi/sentry_wsgi_middleware.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/keystone/wsgi/sentry_wsgi_middleware.py b/keystone/wsgi/sentry_wsgi_middleware.py index 7311eb12a8..df9e2afd78 100644 --- a/keystone/wsgi/sentry_wsgi_middleware.py +++ b/keystone/wsgi/sentry_wsgi_middleware.py @@ -4,6 +4,7 @@ from sentry_sdk.utils import BadDsn import sentry_sdk from sentry_sdk.integrations import wsgi +from sentry_sdk.scrubber import EventScrubber, DEFAULT_DENYLIST from keystone.server.wsgi import initialize_public_application from keystone.common.sentry_filter_engine import SentryFilterEngine @@ -12,6 +13,7 @@ logger = logging.getLogger(__name__) filter_engine = None +denylist = DEFAULT_DENYLIST + ['old_password', 'new_password', 'password', 'cred', 'secret', 'passwd', 'credentials'] # Environment variable configuration # SENTRY_FILTER_CONFIG_FILE: Path to YAML file with filtering rules @@ -62,7 +64,14 @@ def _initialize_sentry_sdk(): return try: - sentry_sdk.init(dsn=sentry_dsn, debug=True, before_send=before_send) + sentry_sdk.init( + dsn=sentry_dsn, + send_default_pii=False, + event_scrubber=EventScrubber(denylist=denylist), + before_send=before_send, + debug=True + ) + logger.info("Sentry SDK initialized successfully") except BadDsn as e: logger.error("Bad SENTRY_DSN format: %s", e) From 2ffce733283167797a6e52ed8f399f6f72eeb76e Mon Sep 17 00:00:00 2001 From: Marcus Loeper Date: Fri, 24 Oct 2025 16:50:58 +0200 Subject: [PATCH 10/15] Refine Sentry filtering rules documentation and remove obsolete exception type --- sentry_filters_example.yaml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/sentry_filters_example.yaml b/sentry_filters_example.yaml index 6539a344cd..007d9a0fd5 100644 --- a/sentry_filters_example.yaml +++ b/sentry_filters_example.yaml @@ -5,7 +5,12 @@ # - exception_type: Filter by exception class name # - message_pattern: Filter by regex pattern in exception message # - message_contains: Filter by simple string in exception message -# - max_occurrences + time_window: Rate limiting (both required together) +# - rate_limit: Limit occurrences of specific exceptions over a time window +# +# within rate_limit: +# - max_occurrences: Maximum number of times the exception can be sent to Sentry within the time window +# - time_window: Time frame in seconds during which max_occurrences is counted +# Example: A rate_limit of max_occurrences: 5 and time_window: 300 means that only 5 occurrences of the exception will be sent to Sentry every 5 minutes. # # Environment variable to set path: SENTRY_FILTER_CONFIG=/path/to/this/file.yaml @@ -14,9 +19,6 @@ rules: - name: "exclude_unauthorized_errors" exception_type: "Unauthorized" - - name: "exclude_ldap_invalid_credentials" - exception_type: "LDAPInvalidCredentialsError" - - name: "exclude_token_not_found" exception_type: "TokenNotFound" @@ -84,3 +86,4 @@ rules: # 7. Consider different time_window values based on issue frequency: # - Short windows (60-300s) for high-frequency issues # - Long windows (600-3600s) for occasional issues you want to limit + From 56811d0c4e224349b97d494b4441195153e8ffe7 Mon Sep 17 00:00:00 2001 From: Marcus Loeper Date: Fri, 24 Oct 2025 17:55:06 +0200 Subject: [PATCH 11/15] fix pep8 --- keystone/common/sentry_filter_engine.py | 217 ++++++++++++----- keystone/common/sentry_rule_loader.py | 123 ++++++---- .../unit/common/test_sentry_filter_engine.py | 96 +++++--- .../unit/common/test_sentry_rule_loader.py | 224 ++++++++++++------ keystone/wsgi/sentry_wsgi_middleware.py | 97 ++++++-- 5 files changed, 512 insertions(+), 245 deletions(-) diff --git a/keystone/common/sentry_filter_engine.py b/keystone/common/sentry_filter_engine.py index e3424279b3..ab5e1543c5 100644 --- a/keystone/common/sentry_filter_engine.py +++ b/keystone/common/sentry_filter_engine.py @@ -10,39 +10,47 @@ # License for the specific language governing permissions and limitations # under the License. +from collections import defaultdict +from collections import deque +import logging import re import time -import logging -from collections import defaultdict, deque -from typing import Dict, List, Any, Optional +from typing import Any +from typing import Dict +from typing import List LOG = logging.getLogger(__name__) class SentryFilterEngine: """Engine for filtering Sentry events based on configurable rules.""" - - def __init__(self, rules: List[Dict[str, Any]], debug_logging: bool = False): + + def __init__( + self, + rules: List[Dict[str, Any]], + debug_logging: bool = False, + ): """Initialize the filter engine with rules. - + Args: rules: List of validated rule dictionaries - debug_logging: Enable detailed debug logging for filtering decisions + debug_logging: Enable detailed debug logging + for filtering decisions """ self.rules = rules self.debug_logging = debug_logging self._rate_limit_tracker = defaultdict(deque) self._compiled_regexes = {} - + LOG.info("Initialized Sentry filter engine with %d rules", len(rules)) self.debug_log("Debug logging enabled for Sentry filtering") - + def should_filter_event(self, event: Dict[str, Any]) -> bool: """Determine if an event should be filtered out. - + Args: event: Sentry event dictionary - + Returns: True if event should be filtered out (not sent to Sentry) False if event should be sent to Sentry @@ -50,7 +58,7 @@ def should_filter_event(self, event: Dict[str, Any]) -> bool: if not self.rules: self.debug_log("No rules configured, allowing event") return False - + for rule in self.rules: if self._rule_matches(rule, event): rule_name = rule.get('name', 'unnamed') @@ -60,35 +68,92 @@ def should_filter_event(self, event: Dict[str, Any]) -> bool: self.debug_log("No rules matched, allowing event") return False - - def _rule_matches(self, rule: Dict[str, Any], event: Dict[str, Any]) -> bool: - """Check if all conditions in a rule match the event.""" + + def _rule_matches( + self, + rule: Dict[str, Any], + event: Dict[str, Any], + ) -> bool: + """Check if all conditions in a rule match the event. + + Each condition present in the rule must match the event. If any + condition does not match the event, the rule does not apply. + """ rule_name = rule.get('name', 'unnamed') - - conditions = [ - ('exception_type', lambda: self._match_exception_type(rule['exception_type'], event)), - ('message_pattern', lambda: self._match_message_pattern(rule['message_pattern'], event)), - ('message_contains', lambda: self._match_message_contains(rule['message_contains'], event)), - ('rate_limit', lambda: self._check_rate_limit(rule, event)), - ] - - for condition_key, checker in conditions: - if condition_key in rule: - if checker(): - self.debug_log("Rule '%s': %s condition matched", rule_name, condition_key) - else: - self.debug_log("Rule '%s': %s condition did not match", rule_name, condition_key) - return False - - return True - - def _match_exception_type(self, rule_type: str, event: Dict[str, Any]) -> bool: + + # exception_type + if 'exception_type' in rule: + ok = self._match_exception_type(rule['exception_type'], event) + if ok: + self.debug_log( + "Rule '%s': exception_type condition matched", + rule_name, + ) + else: + self.debug_log( + "Rule '%s': exception_type condition did not match", + rule_name, + ) + return False + + # message_pattern + if 'message_pattern' in rule: + ok = self._match_message_pattern(rule['message_pattern'], event) + if ok: + self.debug_log( + "Rule '%s': message_pattern condition matched", + rule_name, + ) + else: + self.debug_log( + "Rule '%s': message_pattern condition did not match", + rule_name, + ) + return False + + # message_contains + if 'message_contains' in rule: + ok = self._match_message_contains(rule['message_contains'], event) + if ok: + self.debug_log( + "Rule '%s': message_contains condition matched", + rule_name, + ) + else: + self.debug_log( + "Rule '%s': message_contains condition did not match", + rule_name, + ) + return False + + # rate_limit + if 'rate_limit' in rule: + ok = self._check_rate_limit(rule, event) + if ok: + self.debug_log( + "Rule '%s': rate_limit condition matched", + rule_name, + ) + else: + self.debug_log( + "Rule '%s': rate_limit condition did not match", + rule_name, + ) + return False + + return True + + def _match_exception_type( + self, + rule_type: str, + event: Dict[str, Any], + ) -> bool: """Check if event exception type matches rule. - + Args: rule_type: Exception type from rule event: Sentry event dictionary - + Returns: True if exception type matches """ @@ -97,37 +162,45 @@ def _match_exception_type(self, rule_type: str, event: Dict[str, Any]) -> bool: if exc_value.get('type') == rule_type: return True return False - - def _match_message_pattern(self, pattern: str, event: Dict[str, Any]) -> bool: + + def _match_message_pattern( + self, + pattern: str, + event: Dict[str, Any], + ) -> bool: """Check if event exception message matches regex pattern. - + Args: pattern: Regex pattern string event: Sentry event dictionary - + Returns: True if message matches pattern """ # Cache compiled regex for performance if pattern not in self._compiled_regexes: self._compiled_regexes[pattern] = re.compile(pattern) - + regex = self._compiled_regexes[pattern] exception_values = event.get('exception', {}).get('values', []) - + for exc_value in exception_values: message = exc_value.get('value', '') if regex.search(message): return True return False - - def _match_message_contains(self, search_string: str, event: Dict[str, Any]) -> bool: + + def _match_message_contains( + self, + search_string: str, + event: Dict[str, Any], + ) -> bool: """Check if event exception message contains string. - + Args: search_string: String to search for event: Sentry event dictionary - + Returns: True if message contains string """ @@ -137,57 +210,71 @@ def _match_message_contains(self, search_string: str, event: Dict[str, Any]) -> if search_string in message: return True return False - - def _check_rate_limit(self, rule: Dict[str, Any], event: Dict[str, Any]) -> bool: + + def _check_rate_limit( + self, + rule: Dict[str, Any], + event: Dict[str, Any], + ) -> bool: """Check if rule should apply based on rate limiting. - + Args: rule: Rule dictionary with rate limiting parameters event: Sentry event dictionary - + Returns: True if rule should apply (event should be filtered) False if not at rate limit yet (event should not be filtered) """ rule_name = rule.get('name', 'unnamed') current_time = time.time() - + rate_limit = rule['rate_limit'] occurrences = self._rate_limit_tracker[rule_name] cutoff_time = current_time - rate_limit['time_window'] - + # Clean old entries removed_count = 0 while occurrences and occurrences[0] < cutoff_time: occurrences.popleft() removed_count += 1 - + if self.debug_logging and removed_count > 0: - LOG.debug("Rule '%s': cleaned %d old occurrences", rule_name, removed_count) - + LOG.debug( + "Rule '%s': cleaned %d old occurrences", + rule_name, + removed_count, + ) + # Check if we've hit the limit current_count = len(occurrences) max_occurrences = rate_limit['max_occurrences'] - + if current_count >= max_occurrences: if self.debug_logging: - LOG.debug("Rule '%s': rate limit reached (%d >= %d)", - rule_name, current_count, max_occurrences) + LOG.debug( + "Rule '%s': rate limit reached (%d >= %d)", + rule_name, current_count, max_occurrences + ) return True # Rule applies - filter this event - + # Add current occurrence occurrences.append(current_time) - + if self.debug_logging: - LOG.debug("Rule '%s': added occurrence (%d/%d in %ds window)", - rule_name, current_count + 1, max_occurrences, rate_limit['time_window']) - - return False # Not at limit yet - don't filter + LOG.debug( + "Rule '%s': added occurrence (%d/%d in %ds window)", + rule_name, + current_count + 1, + max_occurrences, + rate_limit['time_window'], + ) + return False # Not at limit yet - don't filter def debug_log(self, *args: Any) -> None: """Log a debug message if debug logging is enabled. - + Args: message: Message to log """ diff --git a/keystone/common/sentry_rule_loader.py b/keystone/common/sentry_rule_loader.py index 73cc9ef5b9..01dc5b8f06 100644 --- a/keystone/common/sentry_rule_loader.py +++ b/keystone/common/sentry_rule_loader.py @@ -10,56 +10,63 @@ # License for the specific language governing permissions and limitations # under the License. +import logging import os +import re import yaml -import logging -from typing import Dict, List, Any + +from typing import Any +from typing import Dict +from typing import List LOG = logging.getLogger(__name__) class RuleValidationError(Exception): """Raised when a rule fails validation.""" + pass def load_rules_from_file(config_file: str) -> List[Dict[str, Any]]: """Load and validate Sentry filtering rules from YAML file. - + Args: config_file: Path to the YAML configuration file - + Returns: List of validated rule dictionaries - + Raises: FileNotFoundError: If config file doesn't exist yaml.YAMLError: If YAML parsing fails RuleValidationError: If rule validation fails """ LOG.info("Loading Sentry filter rules from: %s", config_file) - + if not os.path.exists(config_file): # return empty list if config file does not exist LOG.warning("Sentry filter config file not found: %s", config_file) return [] - + try: with open(config_file, 'r') as f: config_data = yaml.safe_load(f) except yaml.YAMLError as e: - raise yaml.YAMLError(f"Failed to parse YAML config file {config_file}: {e}") - + raise yaml.YAMLError( + f"Failed to parse YAML config file {config_file}: {e}" + ) + if not isinstance(config_data, dict): raise RuleValidationError("Config file must contain a YAML dictionary") - + if 'rules' not in config_data: raise RuleValidationError("Config file must contain a 'rules' section") - + rules = config_data['rules'] if not isinstance(rules, list): raise RuleValidationError("'rules' section must be a list") - + validated_rules = [] for i, rule in enumerate(rules): try: @@ -68,91 +75,113 @@ def load_rules_from_file(config_file: str) -> List[Dict[str, Any]]: except RuleValidationError as e: LOG.error("Rule validation failed for rule %d: %s", i, e) raise RuleValidationError(f"Rule {i} validation failed: {e}") - - LOG.info("Successfully loaded %d Sentry filter rules", len(validated_rules)) + + LOG.info( + "Successfully loaded %d Sentry filter rules", + len(validated_rules), + ) return validated_rules def _validate_rule(rule: Dict[str, Any], rule_index: int) -> Dict[str, Any]: """Validate a single filtering rule. - + Args: rule: Rule dictionary to validate rule_index: Index of rule for error reporting - + Returns: Validated rule dictionary - + Raises: RuleValidationError: If rule validation fails """ if not isinstance(rule, dict): - raise RuleValidationError(f"Rule must be a dictionary, got {type(rule)}") - + raise RuleValidationError( + f"Rule must be a dictionary, got {type(rule)}" + ) + # Rule name is required for logging and rate limiting identification if 'name' not in rule: raise RuleValidationError("Rule must have a 'name' field") - + if not isinstance(rule['name'], str) or not rule['name'].strip(): raise RuleValidationError("Rule 'name' must be a non-empty string") - + # At least one filtering condition must be specified - has_condition = any(field in rule for field in [ - 'exception_type', 'message_pattern', 'message_contains' - ]) - + has_condition = any( + field in rule for field in [ + 'exception_type', 'message_pattern', 'message_contains' + ] + ) + if not has_condition: raise RuleValidationError( "Rule must specify at least one condition: " "'exception_type', 'message_pattern', or 'message_contains'" ) - + # Validate exception_type if 'exception_type' in rule: if not isinstance(rule['exception_type'], str): raise RuleValidationError("'exception_type' must be a string") - + # Validate message_pattern (regex) if 'message_pattern' in rule: if not isinstance(rule['message_pattern'], str): raise RuleValidationError("'message_pattern' must be a string") - + # Test regex compilation try: - import re re.compile(rule['message_pattern']) except re.error as e: - raise RuleValidationError(f"Invalid regex in 'message_pattern': {e}") - + raise RuleValidationError( + f"Invalid regex in 'message_pattern': {e}" + ) + # Validate message_contains if 'message_contains' in rule: if not isinstance(rule['message_contains'], str): raise RuleValidationError("'message_contains' must be a string") - + # Validate rate limiting parameters if 'rate_limit' in rule: rate_limit = rule['rate_limit'] - + if not isinstance(rate_limit, dict): raise RuleValidationError("'rate_limit' must be a dictionary") - - if 'max_occurrences' not in rate_limit or 'time_window' not in rate_limit: + + if ( + 'max_occurrences' not in rate_limit + or 'time_window' not in rate_limit + ): + raise RuleValidationError( + "Rate limiting requires both 'max_occurrences'" + " and 'time_window'" + ) + + if ( + not isinstance(rate_limit['max_occurrences'], int) + or rate_limit['max_occurrences'] <= 0 + ): raise RuleValidationError( - "Rate limiting requires both 'max_occurrences' and 'time_window' in 'rate_limit' section" + "'rate_limit.max_occurrences' must be a positive integer" ) - - if not isinstance(rate_limit['max_occurrences'], int) or rate_limit['max_occurrences'] <= 0: - raise RuleValidationError("'rate_limit.max_occurrences' must be a positive integer") - - if not isinstance(rate_limit['time_window'], (int, float)) or rate_limit['time_window'] <= 0: - raise RuleValidationError("'rate_limit.time_window' must be a positive number") - + + if ( + not isinstance(rate_limit['time_window'], (int, float)) + or rate_limit['time_window'] <= 0 + ): + raise RuleValidationError( + "'rate_limit.time_window' must be a positive number" + ) + return rule def create_example_config_file(output_file: str) -> None: """Create an example configuration file with common filtering rules. - + Args: output_file: Path where to write the example config """ @@ -180,7 +209,7 @@ def create_example_config_file(output_file: str) -> None: }, { 'name': 'filter_connection_refused', - 'message_pattern': 'Connection.*refused.*port\s+\d+' + 'message_pattern': r'Connection.*refused.*port\s+\d+', }, { 'name': 'complex_database_rule', @@ -193,8 +222,8 @@ def create_example_config_file(output_file: str) -> None: } ] } - + with open(output_file, 'w') as f: yaml.dump(example_config, f, default_flow_style=False, indent=2) - + LOG.info("Created example Sentry filter config at: %s", output_file) diff --git a/keystone/tests/unit/common/test_sentry_filter_engine.py b/keystone/tests/unit/common/test_sentry_filter_engine.py index e109ec7565..dfadb39e3a 100644 --- a/keystone/tests/unit/common/test_sentry_filter_engine.py +++ b/keystone/tests/unit/common/test_sentry_filter_engine.py @@ -10,7 +10,6 @@ # License for the specific language governing permissions and limitations # under the License. -import time from unittest import mock from keystone.common.sentry_filter_engine import SentryFilterEngine @@ -27,8 +26,12 @@ def setUp(self): def test_no_rules_allows_all_events(self): """Test that engine with no rules allows all events.""" engine = SentryFilterEngine([]) - event = {'exception': {'values': [{'type': 'TestError', 'value': 'test message'}]}} - + event = { + 'exception': { + 'values': [{'type': 'TestError', 'value': 'test message'}] + } + } + result = engine.should_filter_event(event) self.assertFalse(result) @@ -36,14 +39,18 @@ def test_exception_type_matching(self): """Test basic exception type matching.""" rules = [{'name': 'test_rule', 'exception_type': 'TestError'}] engine = SentryFilterEngine(rules) - + # Matching event should be filtered - matching_event = {'exception': {'values': [{'type': 'TestError', 'value': 'test'}]}} + matching_event = { + 'exception': {'values': [{'type': 'TestError', 'value': 'test'}]} + } result = engine.should_filter_event(matching_event) self.assertTrue(result) - + # Non-matching event should not be filtered - non_matching_event = {'exception': {'values': [{'type': 'OtherError', 'value': 'test'}]}} + non_matching_event = { + 'exception': {'values': [{'type': 'OtherError', 'value': 'test'}]} + } result = engine.should_filter_event(non_matching_event) self.assertFalse(result) @@ -51,14 +58,22 @@ def test_message_contains_matching(self): """Test basic message contains matching.""" rules = [{'name': 'test_rule', 'message_contains': 'timeout'}] engine = SentryFilterEngine(rules) - + # Matching event should be filtered - matching_event = {'exception': {'values': [{'type': 'Error', 'value': 'connection timeout error'}]}} + matching_event = { + 'exception': { + 'values': [{'type': 'Error', + 'value': 'connection timeout error'}] + } + } result = engine.should_filter_event(matching_event) self.assertTrue(result) - + # Non-matching event should not be filtered - non_matching_event = {'exception': {'values': [{'type': 'Error', 'value': 'other error'}]}} + non_matching_event = { + 'exception': {'values': [{'type': 'Error', + 'value': 'other error'}]} + } result = engine.should_filter_event(non_matching_event) self.assertFalse(result) @@ -66,14 +81,20 @@ def test_message_pattern_matching(self): """Test basic regex pattern matching.""" rules = [{'name': 'test_rule', 'message_pattern': 'error.*\\d+'}] engine = SentryFilterEngine(rules) - + # Matching event should be filtered - matching_event = {'exception': {'values': [{'type': 'Error', 'value': 'error code 500'}]}} + matching_event = { + 'exception': {'values': [{'type': 'Error', + 'value': 'error code 500'}]} + } result = engine.should_filter_event(matching_event) self.assertTrue(result) - + # Non-matching event should not be filtered - non_matching_event = {'exception': {'values': [{'type': 'Error', 'value': 'error without number'}]}} + non_matching_event = { + 'exception': {'values': [{'type': 'Error', + 'value': 'error without number'}]} + } result = engine.should_filter_event(non_matching_event) self.assertFalse(result) @@ -85,16 +106,18 @@ def test_rate_limiting_basic_functionality(self): 'rate_limit': {'max_occurrences': 2, 'time_window': 60} }] engine = SentryFilterEngine(rules) - event = {'exception': {'values': [{'type': 'TestError', 'value': 'test'}]}} - + event = { + 'exception': {'values': [{'type': 'TestError', 'value': 'test'}]} + } + # First occurrence should not be filtered (not at limit yet) result = engine.should_filter_event(event) self.assertFalse(result) - + # Second occurrence should not be filtered (not at limit yet) result = engine.should_filter_event(event) self.assertFalse(result) - + # Third occurrence should be filtered (at limit) result = engine.should_filter_event(event) self.assertTrue(result) @@ -108,18 +131,20 @@ def test_rate_limiting_time_window(self, mock_time): 'rate_limit': {'max_occurrences': 1, 'time_window': 10} }] engine = SentryFilterEngine(rules) - event = {'exception': {'values': [{'type': 'TestError', 'value': 'test'}]}} - + event = { + 'exception': {'values': [{'type': 'TestError', 'value': 'test'}]} + } + # First occurrence at time 0 mock_time.return_value = 0 result = engine.should_filter_event(event) self.assertFalse(result) - + # Second occurrence at time 5 (within window) should be filtered mock_time.return_value = 5 result = engine.should_filter_event(event) self.assertTrue(result) - + # Third occurrence at time 15 (outside window) should not be filtered mock_time.return_value = 15 result = engine.should_filter_event(event) @@ -133,14 +158,21 @@ def test_multiple_conditions_must_all_match(self): 'message_contains': 'timeout' }] engine = SentryFilterEngine(rules) - + # Event matching both conditions should be filtered - matching_event = {'exception': {'values': [{'type': 'TestError', 'value': 'timeout error'}]}} + matching_event = { + 'exception': { + 'values': [{'type': 'TestError', 'value': 'timeout error'}] + } + } result = engine.should_filter_event(matching_event) self.assertTrue(result) - + # Event matching only one condition should not be filtered - partial_match_event = {'exception': {'values': [{'type': 'TestError', 'value': 'other error'}]}} + partial_match_event = { + 'exception': {'values': [{'type': 'TestError', + 'value': 'other error'}]} + } result = engine.should_filter_event(partial_match_event) self.assertFalse(result) @@ -148,15 +180,15 @@ def test_get_statistics(self): """Test that statistics can be retrieved.""" rules = [{'name': 'test_rule', 'exception_type': 'TestError'}] engine = SentryFilterEngine(rules) - + stats = engine.get_statistics() - + # Basic structure should be present self.assertIn('total_rules', stats) self.assertIn('rate_limited_rules', stats) self.assertIn('compiled_regexes', stats) self.assertIn('rate_limit_stats', stats) - + # Should have correct rule count self.assertEqual(stats['total_rules'], 1) @@ -164,12 +196,12 @@ def test_empty_event_handling(self): """Test handling of malformed or empty events.""" rules = [{'name': 'test_rule', 'exception_type': 'TestError'}] engine = SentryFilterEngine(rules) - + # Empty event should not be filtered empty_event = {} result = engine.should_filter_event(empty_event) self.assertFalse(result) - + # Event without exception should not be filtered no_exception_event = {'other_field': 'value'} result = engine.should_filter_event(no_exception_event) diff --git a/keystone/tests/unit/common/test_sentry_rule_loader.py b/keystone/tests/unit/common/test_sentry_rule_loader.py index 696da0d067..3cdf3458a0 100644 --- a/keystone/tests/unit/common/test_sentry_rule_loader.py +++ b/keystone/tests/unit/common/test_sentry_rule_loader.py @@ -14,10 +14,8 @@ import tempfile import yaml -from keystone.common.sentry_rule_loader import ( - load_rules_from_file, - RuleValidationError -) +from keystone.common.sentry_rule_loader import load_rules_from_file +from keystone.common.sentry_rule_loader import RuleValidationError from keystone.tests import unit @@ -38,7 +36,9 @@ def tearDown(self): def _create_temp_config(self, config_data): """Create a temporary configuration file with given data.""" - temp_file = tempfile.NamedTemporaryFile(mode='w', suffix='.yaml', delete=False) + temp_file = tempfile.NamedTemporaryFile( + mode='w', suffix='.yaml', delete=False + ) yaml.dump(config_data, temp_file, default_flow_style=False) temp_file.close() self.temp_files.append(temp_file.name) @@ -52,7 +52,7 @@ def test_load_valid_config(self): ] } config_file = self._create_temp_config(config_data) - + # Should not raise any exceptions rules = load_rules_from_file(config_file) self.assertEqual(len(rules), 1) @@ -62,7 +62,7 @@ def test_empty_rules_list(self): """Test configuration with empty rules list.""" config_data = {'rules': []} config_file = self._create_temp_config(config_data) - + rules = load_rules_from_file(config_file) self.assertEqual(len(rules), 0) @@ -70,8 +70,10 @@ def test_missing_rules_section(self): """Test configuration without 'rules' section.""" config_data = {'other_section': []} config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_nonexistent_file(self): """Test loading from non-existent file.""" @@ -81,11 +83,13 @@ def test_nonexistent_file(self): def test_invalid_yaml_format(self): """Test handling of malformed YAML.""" - temp_file = tempfile.NamedTemporaryFile(mode='w', suffix='.yaml', delete=False) + temp_file = tempfile.NamedTemporaryFile( + mode='w', suffix='.yaml', delete=False + ) temp_file.write("invalid: yaml: content: [unclosed") temp_file.close() self.temp_files.append(temp_file.name) - + self.assertRaises(yaml.YAMLError, load_rules_from_file, temp_file.name) def test_rule_not_dictionary(self): @@ -94,8 +98,10 @@ def test_rule_not_dictionary(self): 'rules': ["not_a_dict"] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rule_missing_name(self): """Test validation fails when rule has no 'name' field.""" @@ -103,8 +109,10 @@ def test_rule_missing_name(self): 'rules': [{'exception_type': 'TestError'}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rule_name_not_string(self): """Test validation fails when rule name is not a string.""" @@ -112,8 +120,10 @@ def test_rule_name_not_string(self): 'rules': [{'name': 123, 'exception_type': 'TestError'}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rule_name_empty_string(self): """Test validation fails when rule name is empty.""" @@ -121,8 +131,10 @@ def test_rule_name_empty_string(self): 'rules': [{'name': '', 'exception_type': 'TestError'}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rule_name_whitespace_only(self): """Test validation fails when rule name is only whitespace.""" @@ -130,8 +142,10 @@ def test_rule_name_whitespace_only(self): 'rules': [{'name': ' ', 'exception_type': 'TestError'}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rule_no_conditions(self): """Test validation fails when rule has no filtering conditions.""" @@ -139,8 +153,10 @@ def test_rule_no_conditions(self): 'rules': [{'name': 'test_rule'}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_exception_type_not_string(self): """Test validation fails when exception_type is not a string.""" @@ -148,8 +164,10 @@ def test_exception_type_not_string(self): 'rules': [{'name': 'test_rule', 'exception_type': 123}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_message_pattern_not_string(self): """Test validation fails when message_pattern is not a string.""" @@ -157,8 +175,10 @@ def test_message_pattern_not_string(self): 'rules': [{'name': 'test_rule', 'message_pattern': 123}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_message_pattern_invalid_regex(self): """Test validation fails when message_pattern is invalid regex.""" @@ -166,8 +186,10 @@ def test_message_pattern_invalid_regex(self): 'rules': [{'name': 'test_rule', 'message_pattern': '[unclosed'}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_message_contains_not_string(self): """Test validation fails when message_contains is not a string.""" @@ -175,104 +197,145 @@ def test_message_contains_not_string(self): 'rules': [{'name': 'test_rule', 'message_contains': 123}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rate_limit_not_dictionary(self): """Test validation fails when rate_limit is not a dictionary.""" config_data = { - 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', 'rate_limit': 'not_dict'}] + 'rules': [{'name': 'test_rule', + 'exception_type': 'TestError', + 'rate_limit': 'not_dict'}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rate_limit_missing_max_occurrences(self): """Test validation fails when rate_limit missing max_occurrences.""" config_data = { - 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', 'rate_limit': {'time_window': 300}}] + 'rules': [{'name': 'test_rule', + 'exception_type': 'TestError', + 'rate_limit': {'time_window': 300}}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises(RuleValidationError, + load_rules_from_file, + config_file) def test_rate_limit_missing_time_window(self): """Test validation fails when rate_limit missing time_window.""" config_data = { - 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', 'rate_limit': {'max_occurrences': 5}}] + 'rules': [{'name': 'test_rule', + 'exception_type': 'TestError', + 'rate_limit': {'max_occurrences': 5}}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rate_limit_max_occurrences_not_int(self): """Test validation fails when max_occurrences is not an integer.""" config_data = { - 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', - 'rate_limit': {'max_occurrences': 'five', 'time_window': 300}}] + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': { + 'max_occurrences': 'five', + 'time_window': 300}}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rate_limit_max_occurrences_zero(self): """Test validation fails when max_occurrences is zero.""" config_data = { - 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', - 'rate_limit': {'max_occurrences': 0, 'time_window': 300}}] + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': { + 'max_occurrences': 0, + 'time_window': 300}}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rate_limit_max_occurrences_negative(self): """Test validation fails when max_occurrences is negative.""" config_data = { - 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', - 'rate_limit': {'max_occurrences': -1, 'time_window': 300}}] + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': { + 'max_occurrences': -1, + 'time_window': 300}}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rate_limit_time_window_not_number(self): """Test validation fails when time_window is not a number.""" config_data = { - 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', - 'rate_limit': {'max_occurrences': 5, 'time_window': 'five_minutes'}}] + 'rules': [{'name': 'test_rule', + 'exception_type': 'TestError', + 'rate_limit': { + 'max_occurrences': 5, + 'time_window': 'five_minutes'}}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rate_limit_time_window_zero(self): """Test validation fails when time_window is zero.""" config_data = { - 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', - 'rate_limit': {'max_occurrences': 5, 'time_window': 0}}] + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': { + 'max_occurrences': 5, + 'time_window': 0}}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rate_limit_time_window_negative(self): """Test validation fails when time_window is negative.""" config_data = { - 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', - 'rate_limit': {'max_occurrences': 5, 'time_window': -300}}] + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': { + 'max_occurrences': 5, + 'time_window': -300}}] } config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rate_limit_time_window_float(self): """Test validation succeeds when time_window is a float.""" config_data = { - 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', - 'rate_limit': {'max_occurrences': 5, 'time_window': 300.5}}] + 'rules': [{'name': 'test_rule', 'exception_type': 'TestError', + 'rate_limit': { + 'max_occurrences': 5, + 'time_window': 300.5}}] } config_file = self._create_temp_config(config_data) - + # Should not raise any exceptions rules = load_rules_from_file(config_file) self.assertEqual(len(rules), 1) @@ -280,10 +343,11 @@ def test_rate_limit_time_window_float(self): def test_valid_message_pattern(self): """Test validation succeeds with valid regex pattern.""" config_data = { - 'rules': [{'name': 'test_rule', 'message_pattern': 'Connection.*refused.*port\s+\d+'}] + 'rules': [{'name': 'test_rule', + 'message_pattern': 'Connection.*refused.*port\s+\d+'}] # noqa: W605,E501 } config_file = self._create_temp_config(config_data) - + # Should not raise any exceptions rules = load_rules_from_file(config_file) self.assertEqual(len(rules), 1) @@ -294,13 +358,13 @@ def test_valid_message_contains(self): 'rules': [{'name': 'test_rule', 'message_contains': 'timeout'}] } config_file = self._create_temp_config(config_data) - + # Should not raise any exceptions rules = load_rules_from_file(config_file) self.assertEqual(len(rules), 1) def test_complex_valid_rule(self): - """Test validation succeeds with complex rule having multiple conditions.""" + """Test succeeds with complex rule having multiple conditions.""" config_data = { 'rules': [{ 'name': 'complex_rule', @@ -313,7 +377,7 @@ def test_complex_valid_rule(self): }] } config_file = self._create_temp_config(config_data) - + # Should not raise any exceptions rules = load_rules_from_file(config_file) self.assertEqual(len(rules), 1) @@ -321,16 +385,22 @@ def test_complex_valid_rule(self): def test_config_not_dictionary(self): """Test validation fails when config root is not a dictionary.""" - temp_file = tempfile.NamedTemporaryFile(mode='w', suffix='.yaml', delete=False) + temp_file = tempfile.NamedTemporaryFile( + mode='w', suffix='.yaml', delete=False + ) temp_file.write("- not_a_dict") temp_file.close() self.temp_files.append(temp_file.name) - - self.assertRaises(RuleValidationError, load_rules_from_file, temp_file.name) + + self.assertRaises( + RuleValidationError, load_rules_from_file, temp_file.name + ) def test_rules_not_list(self): """Test validation fails when 'rules' is not a list.""" config_data = {'rules': 'not_a_list'} config_file = self._create_temp_config(config_data) - - self.assertRaises(RuleValidationError, load_rules_from_file, config_file) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) diff --git a/keystone/wsgi/sentry_wsgi_middleware.py b/keystone/wsgi/sentry_wsgi_middleware.py index df9e2afd78..776cf44d21 100644 --- a/keystone/wsgi/sentry_wsgi_middleware.py +++ b/keystone/wsgi/sentry_wsgi_middleware.py @@ -1,52 +1,100 @@ +# 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. + import logging import os -from sentry_sdk.utils import BadDsn import sentry_sdk from sentry_sdk.integrations import wsgi -from sentry_sdk.scrubber import EventScrubber, DEFAULT_DENYLIST +from sentry_sdk.scrubber import DEFAULT_DENYLIST +from sentry_sdk.scrubber import EventScrubber +from sentry_sdk.utils import BadDsn -from keystone.server.wsgi import initialize_public_application from keystone.common.sentry_filter_engine import SentryFilterEngine -from keystone.common.sentry_rule_loader import load_rules_from_file, RuleValidationError +from keystone.common.sentry_rule_loader import load_rules_from_file +from keystone.common.sentry_rule_loader import RuleValidationError +from keystone.server.wsgi import initialize_public_application + logger = logging.getLogger(__name__) filter_engine = None -denylist = DEFAULT_DENYLIST + ['old_password', 'new_password', 'password', 'cred', 'secret', 'passwd', 'credentials'] +denylist = ( + DEFAULT_DENYLIST + + [ + 'old_password', + 'new_password', + 'password', + 'cred', + 'secret', + 'passwd', + 'credentials', + ] +) # Environment variable configuration # SENTRY_FILTER_CONFIG_FILE: Path to YAML file with filtering rules # SENTRY_DEBUG_FILTERING: Enable debug logging for filtering (true/false) # SENTRY_DSN: Sentry DSN for error reporting + def _initialize_sentry_filtering(): - """Initialize the Sentry filtering engine using environment variables.""" + """Initialize the Sentry filtering engine using environment variables. + + This reads SENTRY_FILTER_CONFIG_FILE and SENTRY_DEBUG_FILTERING from the + environment and sets up the global filter engine accordingly. + """ global filter_engine - + sentry_filter_config = os.environ.get("SENTRY_FILTER_CONFIG_FILE") - debug_filtering_str = os.environ.get("SENTRY_DEBUG_FILTERING", "false").lower() + debug_filtering_str = os.environ.get( + "SENTRY_DEBUG_FILTERING", "false" + ).lower() debug_filtering = debug_filtering_str in ("true", "1", "yes", "on") - + if not sentry_filter_config: - logger.info("SENTRY_FILTER_CONFIG_FILE not set, Sentry filtering disabled") + logger.info( + "SENTRY_FILTER_CONFIG_FILE not set, Sentry filtering disabled" + ) return - + if not os.path.exists(sentry_filter_config): - logger.warning("Sentry filter config file not found at %s, filtering disabled", sentry_filter_config) + logger.warning( + "Sentry filter config file not found at %s, filtering disabled", + sentry_filter_config, + ) return try: rules = load_rules_from_file(sentry_filter_config) - filter_engine = SentryFilterEngine(rules, debug_logging=debug_filtering) - logger.info("Loaded %d Sentry filter rules from %s", len(rules), sentry_filter_config) - + filter_engine = SentryFilterEngine( + rules, debug_logging=debug_filtering + ) + logger.info( + "Loaded %d Sentry filter rules from %s", + len(rules), + sentry_filter_config, + ) + except RuleValidationError as e: logger.error("Failed to load Sentry filter config: %s", e) - logger.warning("Sentry filtering disabled due to configuration error") + logger.warning( + "Sentry filtering disabled due to configuration error" + ) except Exception as e: logger.error("Unexpected error loading Sentry filter config: %s", e) - logger.warning("Sentry filtering disabled due to unexpected error") + logger.warning( + "Sentry filtering disabled due to unexpected error" + ) def before_send(event, hint): @@ -57,19 +105,21 @@ def before_send(event, hint): def _initialize_sentry_sdk(): - """Initialize the Sentry SDK with configuration from environment variables.""" + """Initialize Sentry SDK with config from environment variables.""" sentry_dsn = os.environ.get("SENTRY_DSN") if not sentry_dsn: - logger.warning("SENTRY_DSN not found in environment, Sentry disabled") + logger.warning( + "SENTRY_DSN not found in environment, Sentry disabled" + ) return - + try: sentry_sdk.init( dsn=sentry_dsn, send_default_pii=False, event_scrubber=EventScrubber(denylist=denylist), before_send=before_send, - debug=True + debug=True, ) logger.info("Sentry SDK initialized successfully") @@ -80,11 +130,10 @@ def _initialize_sentry_sdk(): def sentry_wsgi_public_wrapper(): - """Create and return the WSGI application wrapped with Sentry middleware.""" - + """Create WSGI application wrapped with Sentry middleware.""" _initialize_sentry_filtering() _initialize_sentry_sdk() - + application = initialize_public_application() sentry_wsgi_public_app = wsgi.SentryWsgiMiddleware(application) return sentry_wsgi_public_app From 40e0cc396544ec0174bf4ed4df04bd978a3f264b Mon Sep 17 00:00:00 2001 From: Marcus Loeper Date: Fri, 24 Oct 2025 18:38:27 +0200 Subject: [PATCH 12/15] remove stats test --- .../unit/common/test_sentry_filter_engine.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/keystone/tests/unit/common/test_sentry_filter_engine.py b/keystone/tests/unit/common/test_sentry_filter_engine.py index dfadb39e3a..9c11ac3903 100644 --- a/keystone/tests/unit/common/test_sentry_filter_engine.py +++ b/keystone/tests/unit/common/test_sentry_filter_engine.py @@ -176,22 +176,6 @@ def test_multiple_conditions_must_all_match(self): result = engine.should_filter_event(partial_match_event) self.assertFalse(result) - def test_get_statistics(self): - """Test that statistics can be retrieved.""" - rules = [{'name': 'test_rule', 'exception_type': 'TestError'}] - engine = SentryFilterEngine(rules) - - stats = engine.get_statistics() - - # Basic structure should be present - self.assertIn('total_rules', stats) - self.assertIn('rate_limited_rules', stats) - self.assertIn('compiled_regexes', stats) - self.assertIn('rate_limit_stats', stats) - - # Should have correct rule count - self.assertEqual(stats['total_rules'], 1) - def test_empty_event_handling(self): """Test handling of malformed or empty events.""" rules = [{'name': 'test_rule', 'exception_type': 'TestError'}] From 2f200cf2f8fe919912ef852217e9adf04a58cbd6 Mon Sep 17 00:00:00 2001 From: Marcus Loeper Date: Mon, 27 Oct 2025 13:56:18 +0100 Subject: [PATCH 13/15] Refactor Sentry filtering to use hint instead of event for improved clarity and accuracy --- keystone/common/sentry_filter_engine.py | 77 ++++++----- .../unit/common/test_sentry_filter_engine.py | 129 ++++++++---------- .../unit/common/test_sentry_rule_loader.py | 6 +- keystone/wsgi/sentry_wsgi_middleware.py | 2 +- 4 files changed, 104 insertions(+), 110 deletions(-) diff --git a/keystone/common/sentry_filter_engine.py b/keystone/common/sentry_filter_engine.py index ab5e1543c5..0c7aea0864 100644 --- a/keystone/common/sentry_filter_engine.py +++ b/keystone/common/sentry_filter_engine.py @@ -45,11 +45,11 @@ def __init__( LOG.info("Initialized Sentry filter engine with %d rules", len(rules)) self.debug_log("Debug logging enabled for Sentry filtering") - def should_filter_event(self, event: Dict[str, Any]) -> bool: + def should_filter_event(self, hint: Dict[str, Any]) -> bool: """Determine if an event should be filtered out. Args: - event: Sentry event dictionary + hint: Holds the original exception information Returns: True if event should be filtered out (not sent to Sentry) @@ -60,7 +60,7 @@ def should_filter_event(self, event: Dict[str, Any]) -> bool: return False for rule in self.rules: - if self._rule_matches(rule, event): + if self._rule_matches(rule, hint): rule_name = rule.get('name', 'unnamed') self.debug_log("Rule '%s' matched, filtering event", rule_name) LOG.info("Filtered Sentry event by rule: %s", rule_name) @@ -72,7 +72,7 @@ def should_filter_event(self, event: Dict[str, Any]) -> bool: def _rule_matches( self, rule: Dict[str, Any], - event: Dict[str, Any], + hint: Dict[str, Any], ) -> bool: """Check if all conditions in a rule match the event. @@ -83,7 +83,7 @@ def _rule_matches( # exception_type if 'exception_type' in rule: - ok = self._match_exception_type(rule['exception_type'], event) + ok = self._match_exception_type(rule['exception_type'], hint) if ok: self.debug_log( "Rule '%s': exception_type condition matched", @@ -98,7 +98,7 @@ def _rule_matches( # message_pattern if 'message_pattern' in rule: - ok = self._match_message_pattern(rule['message_pattern'], event) + ok = self._match_message_pattern(rule['message_pattern'], hint) if ok: self.debug_log( "Rule '%s': message_pattern condition matched", @@ -113,7 +113,7 @@ def _rule_matches( # message_contains if 'message_contains' in rule: - ok = self._match_message_contains(rule['message_contains'], event) + ok = self._match_message_contains(rule['message_contains'], hint) if ok: self.debug_log( "Rule '%s': message_contains condition matched", @@ -128,7 +128,7 @@ def _rule_matches( # rate_limit if 'rate_limit' in rule: - ok = self._check_rate_limit(rule, event) + ok = self._check_rate_limit(rule) if ok: self.debug_log( "Rule '%s': rate_limit condition matched", @@ -146,7 +146,7 @@ def _rule_matches( def _match_exception_type( self, rule_type: str, - event: Dict[str, Any], + hint: Dict[str, Any], ) -> bool: """Check if event exception type matches rule. @@ -157,43 +157,53 @@ def _match_exception_type( Returns: True if exception type matches """ - exception_values = event.get('exception', {}).get('values', []) - for exc_value in exception_values: - if exc_value.get('type') == rule_type: - return True - return False + if 'exc_info' not in hint: + return False + + exception_instance = hint['exc_info'][1] + + if exception_instance is None: + return False + + type_name = type(exception_instance).__name__ + return type_name == rule_type def _match_message_pattern( self, pattern: str, - event: Dict[str, Any], + hint: Dict[str, Any], ) -> bool: """Check if event exception message matches regex pattern. Args: pattern: Regex pattern string - event: Sentry event dictionary + hint: Sentry event hint dictionary Returns: True if message matches pattern """ + if 'exc_info' not in hint: + return False + + exception_instance = hint['exc_info'][1] + + if exception_instance is None: + return False + # Cache compiled regex for performance if pattern not in self._compiled_regexes: self._compiled_regexes[pattern] = re.compile(pattern) regex = self._compiled_regexes[pattern] - exception_values = event.get('exception', {}).get('values', []) - for exc_value in exception_values: - message = exc_value.get('value', '') - if regex.search(message): - return True - return False + message = str(exception_instance) + + return bool(regex.search(message)) def _match_message_contains( self, search_string: str, - event: Dict[str, Any], + hint: Dict[str, Any], ) -> bool: """Check if event exception message contains string. @@ -204,23 +214,26 @@ def _match_message_contains( Returns: True if message contains string """ - exception_values = event.get('exception', {}).get('values', []) - for exc_value in exception_values: - message = exc_value.get('value', '') - if search_string in message: - return True - return False + if 'exc_info' not in hint: + return False + + exception_instance = hint['exc_info'][1] + + if exception_instance is None: + return False + + message = str(exception_instance) + + return search_string in message def _check_rate_limit( self, - rule: Dict[str, Any], - event: Dict[str, Any], + rule: Dict[str, Any] ) -> bool: """Check if rule should apply based on rate limiting. Args: rule: Rule dictionary with rate limiting parameters - event: Sentry event dictionary Returns: True if rule should apply (event should be filtered) diff --git a/keystone/tests/unit/common/test_sentry_filter_engine.py b/keystone/tests/unit/common/test_sentry_filter_engine.py index 9c11ac3903..2f1f6b01be 100644 --- a/keystone/tests/unit/common/test_sentry_filter_engine.py +++ b/keystone/tests/unit/common/test_sentry_filter_engine.py @@ -22,17 +22,27 @@ class SentryFilterEngineTestCase(unit.BaseTestCase): def setUp(self): """Set up test fixtures.""" super(SentryFilterEngineTestCase, self).setUp() + + def _create_hint_with_exception(self, exception_type_name, message): + """Create a hint structure with a mock exception for testing.""" + # Create a custom exception class with the desired name + class CustomException(Exception): + pass + + CustomException.__name__ = exception_type_name + mock_exception = CustomException(message) + + return { + 'exc_info': (CustomException, mock_exception, None) + } def test_no_rules_allows_all_events(self): """Test that engine with no rules allows all events.""" engine = SentryFilterEngine([]) - event = { - 'exception': { - 'values': [{'type': 'TestError', 'value': 'test message'}] - } - } + + hint = self._create_hint_with_exception('TestError', 'test message') - result = engine.should_filter_event(event) + result = engine.should_filter_event(hint) self.assertFalse(result) def test_exception_type_matching(self): @@ -40,18 +50,14 @@ def test_exception_type_matching(self): rules = [{'name': 'test_rule', 'exception_type': 'TestError'}] engine = SentryFilterEngine(rules) - # Matching event should be filtered - matching_event = { - 'exception': {'values': [{'type': 'TestError', 'value': 'test'}]} - } - result = engine.should_filter_event(matching_event) + # Matching hint should be filtered + matching_hint = self._create_hint_with_exception('TestError', 'test') + result = engine.should_filter_event(matching_hint) self.assertTrue(result) - # Non-matching event should not be filtered - non_matching_event = { - 'exception': {'values': [{'type': 'OtherError', 'value': 'test'}]} - } - result = engine.should_filter_event(non_matching_event) + # Non-matching hint should not be filtered + non_matching_hint = self._create_hint_with_exception('OtherError', 'test') + result = engine.should_filter_event(non_matching_hint) self.assertFalse(result) def test_message_contains_matching(self): @@ -59,22 +65,14 @@ def test_message_contains_matching(self): rules = [{'name': 'test_rule', 'message_contains': 'timeout'}] engine = SentryFilterEngine(rules) - # Matching event should be filtered - matching_event = { - 'exception': { - 'values': [{'type': 'Error', - 'value': 'connection timeout error'}] - } - } - result = engine.should_filter_event(matching_event) + # Matching hint should be filtered + matching_hint = self._create_hint_with_exception('Error', 'connection timeout error') + result = engine.should_filter_event(matching_hint) self.assertTrue(result) - # Non-matching event should not be filtered - non_matching_event = { - 'exception': {'values': [{'type': 'Error', - 'value': 'other error'}]} - } - result = engine.should_filter_event(non_matching_event) + # Non-matching hint should not be filtered + non_matching_hint = self._create_hint_with_exception('Error', 'other error') + result = engine.should_filter_event(non_matching_hint) self.assertFalse(result) def test_message_pattern_matching(self): @@ -82,20 +80,14 @@ def test_message_pattern_matching(self): rules = [{'name': 'test_rule', 'message_pattern': 'error.*\\d+'}] engine = SentryFilterEngine(rules) - # Matching event should be filtered - matching_event = { - 'exception': {'values': [{'type': 'Error', - 'value': 'error code 500'}]} - } - result = engine.should_filter_event(matching_event) + # Matching hint should be filtered + matching_hint = self._create_hint_with_exception('Error', 'error code 500') + result = engine.should_filter_event(matching_hint) self.assertTrue(result) - # Non-matching event should not be filtered - non_matching_event = { - 'exception': {'values': [{'type': 'Error', - 'value': 'error without number'}]} - } - result = engine.should_filter_event(non_matching_event) + # Non-matching hint should not be filtered + non_matching_hint = self._create_hint_with_exception('Error', 'error without number') + result = engine.should_filter_event(non_matching_hint) self.assertFalse(result) def test_rate_limiting_basic_functionality(self): @@ -106,20 +98,18 @@ def test_rate_limiting_basic_functionality(self): 'rate_limit': {'max_occurrences': 2, 'time_window': 60} }] engine = SentryFilterEngine(rules) - event = { - 'exception': {'values': [{'type': 'TestError', 'value': 'test'}]} - } + hint = self._create_hint_with_exception('TestError', 'test') # First occurrence should not be filtered (not at limit yet) - result = engine.should_filter_event(event) + result = engine.should_filter_event(hint) self.assertFalse(result) # Second occurrence should not be filtered (not at limit yet) - result = engine.should_filter_event(event) + result = engine.should_filter_event(hint) self.assertFalse(result) # Third occurrence should be filtered (at limit) - result = engine.should_filter_event(event) + result = engine.should_filter_event(hint) self.assertTrue(result) @mock.patch('time.time') @@ -131,23 +121,21 @@ def test_rate_limiting_time_window(self, mock_time): 'rate_limit': {'max_occurrences': 1, 'time_window': 10} }] engine = SentryFilterEngine(rules) - event = { - 'exception': {'values': [{'type': 'TestError', 'value': 'test'}]} - } + hint = self._create_hint_with_exception('TestError', 'test') # First occurrence at time 0 mock_time.return_value = 0 - result = engine.should_filter_event(event) + result = engine.should_filter_event(hint) self.assertFalse(result) # Second occurrence at time 5 (within window) should be filtered mock_time.return_value = 5 - result = engine.should_filter_event(event) + result = engine.should_filter_event(hint) self.assertTrue(result) # Third occurrence at time 15 (outside window) should not be filtered mock_time.return_value = 15 - result = engine.should_filter_event(event) + result = engine.should_filter_event(hint) self.assertFalse(result) def test_multiple_conditions_must_all_match(self): @@ -159,34 +147,27 @@ def test_multiple_conditions_must_all_match(self): }] engine = SentryFilterEngine(rules) - # Event matching both conditions should be filtered - matching_event = { - 'exception': { - 'values': [{'type': 'TestError', 'value': 'timeout error'}] - } - } - result = engine.should_filter_event(matching_event) + # Hint matching both conditions should be filtered + matching_hint = self._create_hint_with_exception('TestError', 'timeout error') + result = engine.should_filter_event(matching_hint) self.assertTrue(result) - # Event matching only one condition should not be filtered - partial_match_event = { - 'exception': {'values': [{'type': 'TestError', - 'value': 'other error'}]} - } - result = engine.should_filter_event(partial_match_event) + # Hint matching only one condition should not be filtered + partial_match_hint = self._create_hint_with_exception('TestError', 'other error') + result = engine.should_filter_event(partial_match_hint) self.assertFalse(result) def test_empty_event_handling(self): - """Test handling of malformed or empty events.""" + """Test handling of malformed or empty hints.""" rules = [{'name': 'test_rule', 'exception_type': 'TestError'}] engine = SentryFilterEngine(rules) - # Empty event should not be filtered - empty_event = {} - result = engine.should_filter_event(empty_event) + # Empty hint should not be filtered + empty_hint = {} + result = engine.should_filter_event(empty_hint) self.assertFalse(result) - # Event without exception should not be filtered - no_exception_event = {'other_field': 'value'} - result = engine.should_filter_event(no_exception_event) + # Hint without exc_info should not be filtered + no_exception_hint = {'other_field': 'value'} + result = engine.should_filter_event(no_exception_hint) self.assertFalse(result) diff --git a/keystone/tests/unit/common/test_sentry_rule_loader.py b/keystone/tests/unit/common/test_sentry_rule_loader.py index 3cdf3458a0..7880d5aee3 100644 --- a/keystone/tests/unit/common/test_sentry_rule_loader.py +++ b/keystone/tests/unit/common/test_sentry_rule_loader.py @@ -224,9 +224,9 @@ def test_rate_limit_missing_max_occurrences(self): } config_file = self._create_temp_config(config_data) - self.assertRaises(RuleValidationError, - load_rules_from_file, - config_file) + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) def test_rate_limit_missing_time_window(self): """Test validation fails when rate_limit missing time_window.""" diff --git a/keystone/wsgi/sentry_wsgi_middleware.py b/keystone/wsgi/sentry_wsgi_middleware.py index 776cf44d21..cba3b36509 100644 --- a/keystone/wsgi/sentry_wsgi_middleware.py +++ b/keystone/wsgi/sentry_wsgi_middleware.py @@ -99,7 +99,7 @@ def _initialize_sentry_filtering(): def before_send(event, hint): """Sentry before_send hook to filter events based on configured rules.""" - if filter_engine and filter_engine.should_filter_event(event): + if filter_engine and filter_engine.should_filter_event(hint): return None return event From 921f1f5b97f62565b44cb6d7d2685367c1af8efb Mon Sep 17 00:00:00 2001 From: Marcus Loeper Date: Wed, 5 Nov 2025 14:57:18 +0100 Subject: [PATCH 14/15] fix pep8 linting errors --- .../unit/common/test_sentry_filter_engine.py | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/keystone/tests/unit/common/test_sentry_filter_engine.py b/keystone/tests/unit/common/test_sentry_filter_engine.py index 2f1f6b01be..985655ffa6 100644 --- a/keystone/tests/unit/common/test_sentry_filter_engine.py +++ b/keystone/tests/unit/common/test_sentry_filter_engine.py @@ -22,16 +22,16 @@ class SentryFilterEngineTestCase(unit.BaseTestCase): def setUp(self): """Set up test fixtures.""" super(SentryFilterEngineTestCase, self).setUp() - + def _create_hint_with_exception(self, exception_type_name, message): """Create a hint structure with a mock exception for testing.""" # Create a custom exception class with the desired name class CustomException(Exception): pass - + CustomException.__name__ = exception_type_name mock_exception = CustomException(message) - + return { 'exc_info': (CustomException, mock_exception, None) } @@ -39,7 +39,7 @@ class CustomException(Exception): def test_no_rules_allows_all_events(self): """Test that engine with no rules allows all events.""" engine = SentryFilterEngine([]) - + hint = self._create_hint_with_exception('TestError', 'test message') result = engine.should_filter_event(hint) @@ -56,7 +56,9 @@ def test_exception_type_matching(self): self.assertTrue(result) # Non-matching hint should not be filtered - non_matching_hint = self._create_hint_with_exception('OtherError', 'test') + non_matching_hint = self._create_hint_with_exception( + 'OtherError', 'test' + ) result = engine.should_filter_event(non_matching_hint) self.assertFalse(result) @@ -66,12 +68,16 @@ def test_message_contains_matching(self): engine = SentryFilterEngine(rules) # Matching hint should be filtered - matching_hint = self._create_hint_with_exception('Error', 'connection timeout error') + matching_hint = self._create_hint_with_exception( + 'Error', 'connection timeout error' + ) result = engine.should_filter_event(matching_hint) self.assertTrue(result) # Non-matching hint should not be filtered - non_matching_hint = self._create_hint_with_exception('Error', 'other error') + non_matching_hint = self._create_hint_with_exception( + 'Error', 'other error' + ) result = engine.should_filter_event(non_matching_hint) self.assertFalse(result) @@ -81,12 +87,16 @@ def test_message_pattern_matching(self): engine = SentryFilterEngine(rules) # Matching hint should be filtered - matching_hint = self._create_hint_with_exception('Error', 'error code 500') + matching_hint = self._create_hint_with_exception( + 'Error', 'error code 500' + ) result = engine.should_filter_event(matching_hint) self.assertTrue(result) # Non-matching hint should not be filtered - non_matching_hint = self._create_hint_with_exception('Error', 'error without number') + non_matching_hint = self._create_hint_with_exception( + 'Error', 'error without number' + ) result = engine.should_filter_event(non_matching_hint) self.assertFalse(result) @@ -148,12 +158,16 @@ def test_multiple_conditions_must_all_match(self): engine = SentryFilterEngine(rules) # Hint matching both conditions should be filtered - matching_hint = self._create_hint_with_exception('TestError', 'timeout error') + matching_hint = self._create_hint_with_exception( + 'TestError', 'timeout error' + ) result = engine.should_filter_event(matching_hint) self.assertTrue(result) # Hint matching only one condition should not be filtered - partial_match_hint = self._create_hint_with_exception('TestError', 'other error') + partial_match_hint = self._create_hint_with_exception( + 'TestError', 'other error' + ) result = engine.should_filter_event(partial_match_hint) self.assertFalse(result) From cb2ed3051183ab6935042f84034e8862c05b4f15 Mon Sep 17 00:00:00 2001 From: Marcus Loeper Date: Tue, 18 Nov 2025 10:37:55 +0100 Subject: [PATCH 15/15] Add sampling mechanism to Sentry filtering and validation for sample_rate --- keystone/common/sentry_filter_engine.py | 23 ++++++++++++++++ keystone/common/sentry_rule_loader.py | 11 +++++++- .../unit/common/test_sentry_filter_engine.py | 23 ++++++++++++++++ .../unit/common/test_sentry_rule_loader.py | 27 +++++++++++++++++++ 4 files changed, 83 insertions(+), 1 deletion(-) diff --git a/keystone/common/sentry_filter_engine.py b/keystone/common/sentry_filter_engine.py index 0c7aea0864..5fd781812d 100644 --- a/keystone/common/sentry_filter_engine.py +++ b/keystone/common/sentry_filter_engine.py @@ -14,6 +14,7 @@ from collections import deque import logging import re +import secrets import time from typing import Any from typing import Dict @@ -141,6 +142,28 @@ def _rule_matches( ) return False + # sample_rate + if 'sample_rate' in rule: + sample_rate = rule['sample_rate'] + rand_value = secrets.SystemRandom().random() + if rand_value <= sample_rate: + self.debug_log( + "Rule '%s': sample_rate condition" + " matched (%.4f <= %.4f)", + rule_name, + rand_value, + sample_rate, + ) + else: + self.debug_log( + "Rule '%s': sample_rate condition" + " did not match (%.4f > %.4f)", + rule_name, + rand_value, + sample_rate, + ) + return False + return True def _match_exception_type( diff --git a/keystone/common/sentry_rule_loader.py b/keystone/common/sentry_rule_loader.py index 01dc5b8f06..019011bd77 100644 --- a/keystone/common/sentry_rule_loader.py +++ b/keystone/common/sentry_rule_loader.py @@ -175,6 +175,14 @@ def _validate_rule(rule: Dict[str, Any], rule_index: int) -> Dict[str, Any]: raise RuleValidationError( "'rate_limit.time_window' must be a positive number" ) + # Validate sample parameters + if 'sample_rate' in rule: + sample_rate = rule['sample_rate'] + if not isinstance(sample_rate, (int, float)): + raise RuleValidationError("'sample_rate' must be a number") + + if not (0.0 < sample_rate <= 1.0): + raise RuleValidationError("'sample_rate' must be between 0 and 1") return rule @@ -205,7 +213,8 @@ def create_example_config_file(output_file: str) -> None: }, { 'name': 'filter_timeout_messages', - 'message_contains': 'timeout' + 'message_contains': 'timeout', + 'sample_rate': 0.5 }, { 'name': 'filter_connection_refused', diff --git a/keystone/tests/unit/common/test_sentry_filter_engine.py b/keystone/tests/unit/common/test_sentry_filter_engine.py index 985655ffa6..9203023900 100644 --- a/keystone/tests/unit/common/test_sentry_filter_engine.py +++ b/keystone/tests/unit/common/test_sentry_filter_engine.py @@ -148,6 +148,29 @@ def test_rate_limiting_time_window(self, mock_time): result = engine.should_filter_event(hint) self.assertFalse(result) + def test_sampling_mechanism(self): + """Test that sampling mechanism works as expected.""" + rules = [{ + 'name': 'sampled_rule', + 'exception_type': 'TestError', + 'sample_rate': 0.5 + }] + engine = SentryFilterEngine(rules) + hint = self._create_hint_with_exception('TestError', 'test') + + # Patch random.random to control sampling outcome + with mock.patch('keystone.common.sentry_filter_' + 'engine.secrets.SystemRandom.random') as mock_random: + # Simulate random value below sample rate (should filter) + mock_random.return_value = 0.4 + result = engine.should_filter_event(hint) + self.assertTrue(result) + + # Simulate random value above sample rate (should not filter) + mock_random.return_value = 0.6 + result = engine.should_filter_event(hint) + self.assertFalse(result) + def test_multiple_conditions_must_all_match(self): """Test that rules with multiple conditions require all to match.""" rules = [{ diff --git a/keystone/tests/unit/common/test_sentry_rule_loader.py b/keystone/tests/unit/common/test_sentry_rule_loader.py index 7880d5aee3..107e14021e 100644 --- a/keystone/tests/unit/common/test_sentry_rule_loader.py +++ b/keystone/tests/unit/common/test_sentry_rule_loader.py @@ -340,6 +340,33 @@ def test_rate_limit_time_window_float(self): rules = load_rules_from_file(config_file) self.assertEqual(len(rules), 1) + def test_sample_rate_not_number(self): + """Test validation fails when sample_rate is not a number.""" + config_data = { + 'rules': [{'name': 'test_rule', + 'exception_type': 'TestError', + 'sample_rate': 'high'}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) + + def test_sample_rate_out_of_bounds(self): + """Test validation fails when sample_rate is out of (0,1] range.""" + for invalid_rate in [-0.1, 1.5, 2]: + config_data = { + 'rules': [{'name': 'test_rule', + 'exception_type': 'TestError', + 'sample_rate': invalid_rate}] + } + config_file = self._create_temp_config(config_data) + + self.assertRaises( + RuleValidationError, load_rules_from_file, config_file + ) + def test_valid_message_pattern(self): """Test validation succeeds with valid regex pattern.""" config_data = {