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/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 diff --git a/keystone/common/sentry_filter_engine.py b/keystone/common/sentry_filter_engine.py new file mode 100644 index 0000000000..5fd781812d --- /dev/null +++ b/keystone/common/sentry_filter_engine.py @@ -0,0 +1,318 @@ +# 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. + +from collections import defaultdict +from collections import deque +import logging +import re +import secrets +import time +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, + ): + """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, hint: Dict[str, Any]) -> bool: + """Determine if an event should be filtered out. + + Args: + hint: Holds the original exception information + + 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, 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) + return True + + self.debug_log("No rules matched, allowing event") + return False + + def _rule_matches( + self, + rule: Dict[str, Any], + hint: 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') + + # exception_type + if 'exception_type' in rule: + ok = self._match_exception_type(rule['exception_type'], hint) + 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'], hint) + 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'], hint) + 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) + 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 + + # 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( + self, + rule_type: str, + hint: 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 + """ + 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, + hint: Dict[str, Any], + ) -> bool: + """Check if event exception message matches regex pattern. + + Args: + pattern: Regex pattern string + 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] + + message = str(exception_instance) + + return bool(regex.search(message)) + + def _match_message_contains( + self, + search_string: str, + hint: 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 + """ + 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] + ) -> bool: + """Check if rule should apply based on rate limiting. + + Args: + rule: Rule dictionary with rate limiting parameters + + 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..019011bd77 --- /dev/null +++ b/keystone/common/sentry_rule_loader.py @@ -0,0 +1,238 @@ +# 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 +import re +import yaml + +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}" + ) + + 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: + 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'" + ) + + 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" + ) + # 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 + + +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', + 'sample_rate': 0.5 + }, + { + 'name': 'filter_connection_refused', + 'message_pattern': r'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/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 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..9203023900 --- /dev/null +++ b/keystone/tests/unit/common/test_sentry_filter_engine.py @@ -0,0 +1,210 @@ +# 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. + +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 _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([]) + + hint = self._create_hint_with_exception('TestError', 'test message') + + result = engine.should_filter_event(hint) + 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 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 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): + """Test basic message contains matching.""" + rules = [{'name': 'test_rule', 'message_contains': 'timeout'}] + engine = SentryFilterEngine(rules) + + # 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 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): + """Test basic regex pattern matching.""" + rules = [{'name': 'test_rule', 'message_pattern': 'error.*\\d+'}] + engine = SentryFilterEngine(rules) + + # 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 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): + """Test basic rate limiting functionality.""" + rules = [{ + 'name': 'rate_limited_rule', + 'exception_type': 'TestError', + 'rate_limit': {'max_occurrences': 2, 'time_window': 60} + }] + engine = SentryFilterEngine(rules) + hint = self._create_hint_with_exception('TestError', 'test') + + # First occurrence should not be filtered (not at limit yet) + result = engine.should_filter_event(hint) + self.assertFalse(result) + + # Second occurrence should not be filtered (not at limit yet) + result = engine.should_filter_event(hint) + self.assertFalse(result) + + # Third occurrence should be filtered (at limit) + result = engine.should_filter_event(hint) + 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) + hint = self._create_hint_with_exception('TestError', 'test') + + # First occurrence at time 0 + mock_time.return_value = 0 + 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(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(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 = [{ + 'name': 'multi_condition_rule', + 'exception_type': 'TestError', + 'message_contains': 'timeout' + }] + engine = SentryFilterEngine(rules) + + # 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) + + # 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 hints.""" + rules = [{'name': 'test_rule', 'exception_type': 'TestError'}] + engine = SentryFilterEngine(rules) + + # Empty hint should not be filtered + empty_hint = {} + result = engine.should_filter_event(empty_hint) + self.assertFalse(result) + + # 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 new file mode 100644 index 0000000000..107e14021e --- /dev/null +++ b/keystone/tests/unit/common/test_sentry_rule_loader.py @@ -0,0 +1,433 @@ +# 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 +from keystone.common.sentry_rule_loader import 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_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 = { + '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) + + 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 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 new file mode 100644 index 0000000000..cba3b36509 --- /dev/null +++ b/keystone/wsgi/sentry_wsgi_middleware.py @@ -0,0 +1,142 @@ +# 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 + +import sentry_sdk +from sentry_sdk.integrations import wsgi +from sentry_sdk.scrubber import DEFAULT_DENYLIST +from sentry_sdk.scrubber import EventScrubber +from sentry_sdk.utils import BadDsn + +from keystone.common.sentry_filter_engine import SentryFilterEngine +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', + ] +) + +# 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. + + 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 = 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(hint): + return None + return event + + +def _initialize_sentry_sdk(): + """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" + ) + return + + try: + 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) + except Exception as e: + logger.error("Exception while initializing Sentry SDK: %s", e) + + +def sentry_wsgi_public_wrapper(): + """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 + + +sentry_wsgi_admin_wrapper = sentry_wsgi_public_wrapper diff --git a/requirements.txt b/requirements.txt index 14d7714a20..bad7b3c0fb 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==2.27.0 # MIT diff --git a/sentry_filters_example.yaml b/sentry_filters_example.yaml new file mode 100644 index 0000000000..007d9a0fd5 --- /dev/null +++ b/sentry_filters_example.yaml @@ -0,0 +1,89 @@ +# 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 +# - 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 + +rules: + # Simple exclusion rules - always filter these exceptions + - name: "exclude_unauthorized_errors" + exception_type: "Unauthorized" + + - 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 + 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