diff --git a/aws_lambda_powertools/utilities/feature_flags/__init__.py b/aws_lambda_powertools/utilities/feature_flags/__init__.py index db7dfca5b57..e8d8229c9dc 100644 --- a/aws_lambda_powertools/utilities/feature_flags/__init__.py +++ b/aws_lambda_powertools/utilities/feature_flags/__init__.py @@ -1,4 +1,5 @@ """Advanced feature flags utility""" + from .appconfig import AppConfigStore from .base import StoreProvider from .exceptions import ConfigurationStoreError diff --git a/aws_lambda_powertools/utilities/feature_flags/comparators.py b/aws_lambda_powertools/utilities/feature_flags/comparators.py index 1419dd6dd83..03cb91e649a 100644 --- a/aws_lambda_powertools/utilities/feature_flags/comparators.py +++ b/aws_lambda_powertools/utilities/feature_flags/comparators.py @@ -1,10 +1,11 @@ +from __future__ import annotations + from datetime import datetime, tzinfo from typing import Any, Dict, Optional from dateutil.tz import gettz from .schema import HOUR_MIN_SEPARATOR, ModuloRangeValues, TimeValues -from .exceptions import SchemaValidationError def _get_now_from_timezone(timezone: Optional[tzinfo]) -> datetime: @@ -85,41 +86,64 @@ def compare_modulo_range(context_value: int, condition_value: Dict) -> bool: return start <= context_value % base <= end -def compare_any_in_list(key_list, value_list): - if not (isinstance(key_list, list) and isinstance(value_list, list)): - raise SchemaValidationError() - - results = False - for key in key_list: - if key in value_list: - results = True - break - - return results +def compare_any_in_list(context_value: list, condition_value: list) -> bool: + """Comparator for ANY_IN_VALUE action + + Parameters + ---------- + context_value : list + user-defined context for flag evaluation + condition_value : list + schema value available for condition being evaluated + + Returns + ------- + bool + Whether any list item in context_value is available in condition_value + """ + if not isinstance(context_value, list): + raise ValueError("Context provided must be a list. Unable to compare ANY_IN_VALUE action.") + + return any(key in condition_value for key in context_value) + +def compare_all_in_list(context_value: list, condition_value: list) -> bool: + """Comparator for ALL_IN_VALUE action -def compare_all_in_list(key_list, value_list): - if not (isinstance(key_list, list) and isinstance(value_list, list)): - raise SchemaValidationError() + Parameters + ---------- + context_value : list + user-defined context for flag evaluation + condition_value : list + schema value available for condition being evaluated - results = True - for key in key_list: - if key not in value_list: - results = False - break + Returns + ------- + bool + Whether all list items in context_value are available in condition_value + """ + if not isinstance(context_value, list): + raise ValueError("Context provided must be a list. Unable to compare ALL_IN_VALUE action.") - return results + return all(key in condition_value for key in context_value) -def compare_none_in_list(key_list, value_list): - if not (isinstance(key_list, list) and isinstance(value_list, list)): - raise SchemaValidationError() +def compare_none_in_list(context_value: list, condition_value: list) -> bool: + """Comparator for NONE_IN_VALUE action - results = True - for key in key_list: - if key in value_list: - results = False - break + Parameters + ---------- + context_value : list + user-defined context for flag evaluation + condition_value : list + schema value available for condition being evaluated - return results + Returns + ------- + bool + Whether list items in context_value are **not** available in condition_value + """ + if not isinstance(context_value, list): + raise ValueError("Context provided must be a list. Unable to compare NONE_IN_VALUE action.") + return all(key not in condition_value for key in context_value) diff --git a/aws_lambda_powertools/utilities/feature_flags/feature_flags.py b/aws_lambda_powertools/utilities/feature_flags/feature_flags.py index ac4dfef0162..bd7e19d0efe 100644 --- a/aws_lambda_powertools/utilities/feature_flags/feature_flags.py +++ b/aws_lambda_powertools/utilities/feature_flags/feature_flags.py @@ -1,21 +1,52 @@ +from __future__ import annotations + import logging -from typing import Any, Dict, List, Optional, Union, cast +from typing import Any, Callable, Dict, List, Optional, TypeVar, Union, cast + +from typing_extensions import ParamSpec from ... import Logger from ...shared.types import JSONType from . import schema from .base import StoreProvider from .comparators import ( + compare_all_in_list, + compare_any_in_list, compare_datetime_range, compare_days_of_week, compare_modulo_range, + compare_none_in_list, compare_time_range, - compare_all_in_list, - compare_any_in_list, - compare_none_in_list ) from .exceptions import ConfigurationStoreError +T = TypeVar("T") +P = ParamSpec("P") + +RULE_ACTION_MAPPING = { + schema.RuleAction.EQUALS.value: lambda a, b: a == b, + schema.RuleAction.NOT_EQUALS.value: lambda a, b: a != b, + schema.RuleAction.KEY_GREATER_THAN_VALUE.value: lambda a, b: a > b, + schema.RuleAction.KEY_GREATER_THAN_OR_EQUAL_VALUE.value: lambda a, b: a >= b, + schema.RuleAction.KEY_LESS_THAN_VALUE.value: lambda a, b: a < b, + schema.RuleAction.KEY_LESS_THAN_OR_EQUAL_VALUE.value: lambda a, b: a <= b, + schema.RuleAction.STARTSWITH.value: lambda a, b: a.startswith(b), + schema.RuleAction.ENDSWITH.value: lambda a, b: a.endswith(b), + schema.RuleAction.IN.value: lambda a, b: a in b, + schema.RuleAction.NOT_IN.value: lambda a, b: a not in b, + schema.RuleAction.KEY_IN_VALUE.value: lambda a, b: a in b, + schema.RuleAction.KEY_NOT_IN_VALUE.value: lambda a, b: a not in b, + schema.RuleAction.VALUE_IN_KEY.value: lambda a, b: b in a, + schema.RuleAction.VALUE_NOT_IN_KEY.value: lambda a, b: b not in a, + schema.RuleAction.ALL_IN_VALUE.value: lambda a, b: compare_all_in_list(a, b), + schema.RuleAction.ANY_IN_VALUE.value: lambda a, b: compare_any_in_list(a, b), + schema.RuleAction.NONE_IN_VALUE.value: lambda a, b: compare_none_in_list(a, b), + schema.RuleAction.SCHEDULE_BETWEEN_TIME_RANGE.value: lambda a, b: compare_time_range(a, b), + schema.RuleAction.SCHEDULE_BETWEEN_DATETIME_RANGE.value: lambda a, b: compare_datetime_range(a, b), + schema.RuleAction.SCHEDULE_BETWEEN_DAYS_OF_WEEK.value: lambda a, b: compare_days_of_week(a, b), + schema.RuleAction.MODULO_RANGE.value: lambda a, b: compare_modulo_range(a, b), +} + class FeatureFlags: def __init__(self, store: StoreProvider, logger: Optional[Union[logging.Logger, Logger]] = None): @@ -49,37 +80,20 @@ def __init__(self, store: StoreProvider, logger: Optional[Union[logging.Logger, """ self.store = store self.logger = logger or logging.getLogger(__name__) + self._exception_handlers: dict[Exception, Callable] = {} def _match_by_action(self, action: str, condition_value: Any, context_value: Any) -> bool: - mapping_by_action = { - schema.RuleAction.EQUALS.value: lambda a, b: a == b, - schema.RuleAction.NOT_EQUALS.value: lambda a, b: a != b, - schema.RuleAction.KEY_GREATER_THAN_VALUE.value: lambda a, b: a > b, - schema.RuleAction.KEY_GREATER_THAN_OR_EQUAL_VALUE.value: lambda a, b: a >= b, - schema.RuleAction.KEY_LESS_THAN_VALUE.value: lambda a, b: a < b, - schema.RuleAction.KEY_LESS_THAN_OR_EQUAL_VALUE.value: lambda a, b: a <= b, - schema.RuleAction.STARTSWITH.value: lambda a, b: a.startswith(b), - schema.RuleAction.ENDSWITH.value: lambda a, b: a.endswith(b), - schema.RuleAction.IN.value: lambda a, b: a in b, - schema.RuleAction.NOT_IN.value: lambda a, b: a not in b, - schema.RuleAction.KEY_IN_VALUE.value: lambda a, b: a in b, - schema.RuleAction.KEY_NOT_IN_VALUE.value: lambda a, b: a not in b, - schema.RuleAction.VALUE_IN_KEY.value: lambda a, b: b in a, - schema.RuleAction.VALUE_NOT_IN_KEY.value: lambda a, b: b not in a, - schema.RuleAction.ALL_IN_VALUE.value: lambda a, b: compare_all_in_list(a, b), - schema.RuleAction.ANY_IN_VALUE.value: lambda a, b: compare_any_in_list(a, b), - schema.RuleAction.NONE_IN_VALUE.value: lambda a, b: compare_none_in_list(a, b), - schema.RuleAction.SCHEDULE_BETWEEN_TIME_RANGE.value: lambda a, b: compare_time_range(a, b), - schema.RuleAction.SCHEDULE_BETWEEN_DATETIME_RANGE.value: lambda a, b: compare_datetime_range(a, b), - schema.RuleAction.SCHEDULE_BETWEEN_DAYS_OF_WEEK.value: lambda a, b: compare_days_of_week(a, b), - schema.RuleAction.MODULO_RANGE.value: lambda a, b: compare_modulo_range(a, b), - } - try: - func = mapping_by_action.get(action, lambda a, b: False) + func = RULE_ACTION_MAPPING.get(action, lambda a, b: False) return func(context_value, condition_value) except Exception as exc: self.logger.debug(f"caught exception while matching action: action={action}, exception={str(exc)}") + + handler = self._lookup_exception_handler(exc) + if handler: + self.logger.debug("Exception handler found! Delegating response.") + return handler(exc) + return False def _evaluate_conditions( @@ -209,6 +223,22 @@ def evaluate(self, *, name: str, context: Optional[Dict[str, Any]] = None, defau 2. Feature exists but has either no rules or no match, return feature default value 3. Feature doesn't exist in stored schema, encountered an error when fetching -> return default value provided + ┌────────────────────────┐ ┌────────────────────────┐ ┌────────────────────────┐ + │ Feature flags │──────▶ Get Configuration ├───────▶ Evaluate rules │ + └────────────────────────┘ │ │ │ │ + │┌──────────────────────┐│ │┌──────────────────────┐│ + ││ Fetch schema ││ ││ Match rule ││ + │└───────────┬──────────┘│ │└───────────┬──────────┘│ + │ │ │ │ │ │ + │┌───────────▼──────────┐│ │┌───────────▼──────────┐│ + ││ Cache schema ││ ││ Match condition ││ + │└───────────┬──────────┘│ │└───────────┬──────────┘│ + │ │ │ │ │ │ + │┌───────────▼──────────┐│ │┌───────────▼──────────┐│ + ││ Validate schema ││ ││ Match action ││ + │└──────────────────────┘│ │└──────────────────────┘│ + └────────────────────────┘ └────────────────────────┘ + Parameters ---------- name: str @@ -222,6 +252,31 @@ def evaluate(self, *, name: str, context: Optional[Dict[str, Any]] = None, defau or there has been an error when fetching the configuration from the store Can be boolean or any JSON values for non-boolean features. + + Examples + -------- + + ```python + from aws_lambda_powertools.utilities.feature_flags import AppConfigStore, FeatureFlags + from aws_lambda_powertools.utilities.typing import LambdaContext + + app_config = AppConfigStore(environment="dev", application="product-catalogue", name="features") + + feature_flags = FeatureFlags(store=app_config) + + + def lambda_handler(event: dict, context: LambdaContext): + # Get customer's tier from incoming request + ctx = {"tier": event.get("tier", "standard")} + + # Evaluate whether customer's tier has access to premium features + # based on `has_premium_features` rules + has_premium_features: bool = feature_flags.evaluate(name="premium_features", context=ctx, default=False) + if has_premium_features: + # enable premium features + ... + ``` + Returns ------ JSONType @@ -335,3 +390,45 @@ def get_enabled_features(self, *, context: Optional[Dict[str, Any]] = None) -> L features_enabled.append(name) return features_enabled + + def validation_exception_handler(self, exc_class: Exception | list[Exception]): + """Registers function to handle unexpected validation exceptions when evaluating flags. + + It does not override the function of a default flag value in case of network and IAM permissions. + For example, you won't be able to catch ConfigurationStoreError exception. + + Parameters + ---------- + exc_class : Exception | list[Exception] + One or more exceptions to catch + + Examples + -------- + + ```python + feature_flags = FeatureFlags(store=app_config) + + @feature_flags.validation_exception_handler(Exception) # any exception + def catch_exception(exc): + raise TypeError("re-raised") from exc + ``` + """ + + def register_exception_handler(func: Callable[P, T]) -> Callable[P, T]: + if isinstance(exc_class, list): + for exp in exc_class: + self._exception_handlers[exp] = func + else: + self._exception_handlers[exc_class] = func + + return func + + return register_exception_handler + + def _lookup_exception_handler(self, exc: BaseException) -> Callable | None: + # Use "Method Resolution Order" to allow for matching against a base class + # of an exception + for cls in type(exc).__mro__: + if cls in self._exception_handlers: + return self._exception_handlers[cls] # type: ignore[index] # index is correct + return None diff --git a/aws_lambda_powertools/utilities/feature_flags/schema.py b/aws_lambda_powertools/utilities/feature_flags/schema.py index 2ef4b9e29a4..1df16677bd8 100644 --- a/aws_lambda_powertools/utilities/feature_flags/schema.py +++ b/aws_lambda_powertools/utilities/feature_flags/schema.py @@ -1,8 +1,11 @@ +from __future__ import annotations + import logging import re from datetime import datetime from enum import Enum -from typing import Any, Callable, Dict, List, Optional, Union +from functools import lru_cache +from typing import Any, Dict, List, Optional, Union from dateutil import tz @@ -19,9 +22,11 @@ CONDITION_ACTION = "action" FEATURE_DEFAULT_VAL_TYPE_KEY = "boolean_type" TIME_RANGE_FORMAT = "%H:%M" # hour:min 24 hours clock -TIME_RANGE_RE_PATTERN = re.compile(r"2[0-3]:[0-5]\d|[0-1]\d:[0-5]\d") # 24 hour clock +TIME_RANGE_PATTERN = re.compile(r"2[0-3]:[0-5]\d|[0-1]\d:[0-5]\d") # 24 hour clock HOUR_MIN_SEPARATOR = ":" +LOGGER: logging.Logger | Logger = logging.getLogger(__name__) + class RuleAction(str, Enum): EQUALS = "EQUALS" @@ -74,6 +79,11 @@ class TimeValues(Enum): FRIDAY = "FRIDAY" SATURDAY = "SATURDAY" + @classmethod + @lru_cache(maxsize=1) + def days(cls) -> list[str]: + return [day.value for day in cls if day.value not in ["START", "END", "TIMEZONE"]] + class ModuloRangeValues(Enum): """ @@ -188,7 +198,12 @@ class SchemaValidator(BaseValidator): def __init__(self, schema: Dict[str, Any], logger: Optional[Union[logging.Logger, Logger]] = None): self.schema = schema - self.logger = logger or logging.getLogger(__name__) + self.logger = logger or LOGGER + + # Validators are designed for modular testing + # therefore we link the custom logger with global LOGGER + # so custom validators can use them when necessary + SchemaValidator._link_global_logger(self.logger) def validate(self) -> None: self.logger.debug("Validating schema") @@ -198,13 +213,18 @@ def validate(self) -> None: features = FeaturesValidator(schema=self.schema, logger=self.logger) features.validate() + @staticmethod + def _link_global_logger(logger: logging.Logger | Logger): + global LOGGER + LOGGER = logger + class FeaturesValidator(BaseValidator): """Validates each feature and calls RulesValidator to validate its rules""" def __init__(self, schema: Dict, logger: Optional[Union[logging.Logger, Logger]] = None): self.schema = schema - self.logger = logger or logging.getLogger(__name__) + self.logger = logger or LOGGER def validate(self): for name, feature in self.schema.items(): @@ -242,7 +262,7 @@ def __init__( self.feature = feature self.feature_name = next(iter(self.feature)) self.rules: Optional[Dict] = self.feature.get(RULES_KEY) - self.logger = logger or logging.getLogger(__name__) + self.logger = logger or LOGGER self.boolean_feature = boolean_feature def validate(self): @@ -289,7 +309,7 @@ class ConditionsValidator(BaseValidator): def __init__(self, rule: Dict[str, Any], rule_name: str, logger: Optional[Union[logging.Logger, Logger]] = None): self.conditions: List[Dict[str, Any]] = rule.get(CONDITIONS_KEY, {}) self.rule_name = rule_name - self.logger = logger or logging.getLogger(__name__) + self.logger = logger or LOGGER def validate(self): if not self.conditions or not isinstance(self.conditions, list): @@ -325,23 +345,26 @@ def validate_condition_key(condition: Dict[str, Any], rule_name: str): if not key or not isinstance(key, str): raise SchemaValidationError(f"'key' value must be a non empty string, rule={rule_name}") - # time actions need to have very specific keys - # SCHEDULE_BETWEEN_TIME_RANGE => CURRENT_TIME - # SCHEDULE_BETWEEN_DATETIME_RANGE => CURRENT_DATETIME - # SCHEDULE_BETWEEN_DAYS_OF_WEEK => CURRENT_DAY_OF_WEEK action = condition.get(CONDITION_ACTION, "") - if action == RuleAction.SCHEDULE_BETWEEN_TIME_RANGE.value and key != TimeKeys.CURRENT_TIME.value: - raise SchemaValidationError( - f"'condition with a 'SCHEDULE_BETWEEN_TIME_RANGE' action must have a 'CURRENT_TIME' condition key, rule={rule_name}", # noqa: E501 - ) - if action == RuleAction.SCHEDULE_BETWEEN_DATETIME_RANGE.value and key != TimeKeys.CURRENT_DATETIME.value: - raise SchemaValidationError( - f"'condition with a 'SCHEDULE_BETWEEN_DATETIME_RANGE' action must have a 'CURRENT_DATETIME' condition key, rule={rule_name}", # noqa: E501 - ) - if action == RuleAction.SCHEDULE_BETWEEN_DAYS_OF_WEEK.value and key != TimeKeys.CURRENT_DAY_OF_WEEK.value: - raise SchemaValidationError( - f"'condition with a 'SCHEDULE_BETWEEN_DAYS_OF_WEEK' action must have a 'CURRENT_DAY_OF_WEEK' condition key, rule={rule_name}", # noqa: E501 - ) + + # To allow for growth and prevent if/elif chains, we align extra validators based on the action name. + # for example: + # + # SCHEDULE_BETWEEN_DAYS_OF_WEEK_KEY + # - extra validation: `_validate_schedule_between_days_of_week_key` + # + # maintenance: we should split to separate file/classes for better organization, e.g., visitor pattern. + + custom_validator = getattr(ConditionsValidator, f"_validate_{action.lower()}_key", None) + + # ~90% of actions available don't require a custom validator + # logging a debug statement for no-match will increase CPU cycles for most customers + # for that reason only, we invert and log only when extra validation is found. + if custom_validator is None: + return + + LOGGER.debug(f"{action} requires key validation. Running '{custom_validator}' validator.") + custom_validator(key, rule_name) @staticmethod def validate_condition_value(condition: Dict[str, Any], rule_name: str): @@ -350,65 +373,35 @@ def validate_condition_value(condition: Dict[str, Any], rule_name: str): raise SchemaValidationError(f"'value' key must not be null, rule={rule_name}") action = condition.get(CONDITION_ACTION, "") - # time actions need to be parsed to make sure date and time format is valid and timezone is recognized - if action == RuleAction.SCHEDULE_BETWEEN_TIME_RANGE.value: - ConditionsValidator._validate_schedule_between_time_and_datetime_ranges( - value, - rule_name, - action, - ConditionsValidator._validate_time_value, - ) - elif action == RuleAction.SCHEDULE_BETWEEN_DATETIME_RANGE.value: - ConditionsValidator._validate_schedule_between_time_and_datetime_ranges( - value, - rule_name, - action, - ConditionsValidator._validate_datetime_value, - ) - elif action == RuleAction.SCHEDULE_BETWEEN_DAYS_OF_WEEK.value: - ConditionsValidator._validate_schedule_between_days_of_week(value, rule_name) - # modulo range condition needs validation on base, start, and end attributes - elif action == RuleAction.MODULO_RANGE.value: - ConditionsValidator._validate_modulo_range(value, rule_name) + # To allow for growth and prevent if/elif chains, we align extra validators based on the action name. + # for example: + # + # SCHEDULE_BETWEEN_DAYS_OF_WEEK_KEY + # - extra validation: `_validate_schedule_between_days_of_week_value` + # + # maintenance: we should split to separate file/classes for better organization, e.g., visitor pattern. - @staticmethod - def _validate_datetime_value(datetime_str: str, rule_name: str): - date = None + custom_validator = getattr(ConditionsValidator, f"_validate_{action.lower()}_value", None) - # We try to parse first with timezone information in order to return the correct error messages - # when a timestamp with timezone is used. Otherwise, the user would get the first error "must be a valid - # ISO8601 time format" which is misleading + # ~90% of actions available don't require a custom validator + # logging a debug statement for no-match will increase CPU cycles for most customers + # for that reason only, we invert and log only when extra validation is found. + if custom_validator is None: + return - try: - # python < 3.11 don't support the Z timezone on datetime.fromisoformat, - # so we replace any Z with the equivalent "+00:00" - # datetime.fromisoformat is orders of magnitude faster than datetime.strptime - date = datetime.fromisoformat(datetime_str.replace("Z", "+00:00")) - except Exception: - raise SchemaValidationError(f"'START' and 'END' must be a valid ISO8601 time format, rule={rule_name}") + LOGGER.debug(f"{action} requires value validation. Running '{custom_validator}' validator.") - # we only allow timezone information to be set via the TIMEZONE field - # this way we can encode DST into the calculation. For instance, Copenhagen is - # UTC+2 during winter, and UTC+1 during summer, which would be impossible to define - # using a single ISO datetime string - if date.tzinfo is not None: - raise SchemaValidationError( - "'START' and 'END' must not include timezone information. Set the timezone using the 'TIMEZONE' " - f"field, rule={rule_name} ", - ) + custom_validator(value, rule_name) @staticmethod - def _validate_time_value(time: str, rule_name: str): - # Using a regex instead of strptime because it's several orders of magnitude faster - match = TIME_RANGE_RE_PATTERN.match(time) - - if not match: + def _validate_schedule_between_days_of_week_key(key: str, rule_name: str): + if key != TimeKeys.CURRENT_DAY_OF_WEEK.value: raise SchemaValidationError( - f"'START' and 'END' must be a valid time format, time_format={TIME_RANGE_FORMAT}, rule={rule_name}", + f"'condition with a 'SCHEDULE_BETWEEN_DAYS_OF_WEEK' action must have a 'CURRENT_DAY_OF_WEEK' condition key, rule={rule_name}", # noqa: E501 ) @staticmethod - def _validate_schedule_between_days_of_week(value: Any, rule_name: str): + def _validate_schedule_between_days_of_week_value(value: dict, rule_name: str): error_str = f"condition with a CURRENT_DAY_OF_WEEK action must have a condition value dictionary with 'DAYS' and 'TIMEZONE' (optional) keys, rule={rule_name}" # noqa: E501 if not isinstance(value, dict): raise SchemaValidationError(error_str) @@ -416,59 +409,70 @@ def _validate_schedule_between_days_of_week(value: Any, rule_name: str): days = value.get(TimeValues.DAYS.value) if not isinstance(days, list) or not value: raise SchemaValidationError(error_str) + + valid_days = TimeValues.days() for day in days: - if not isinstance(day, str) or day not in [ - TimeValues.MONDAY.value, - TimeValues.TUESDAY.value, - TimeValues.WEDNESDAY.value, - TimeValues.THURSDAY.value, - TimeValues.FRIDAY.value, - TimeValues.SATURDAY.value, - TimeValues.SUNDAY.value, - ]: + if not isinstance(day, str) or day not in valid_days: raise SchemaValidationError( f"condition value DAYS must represent a day of the week in 'TimeValues' enum, rule={rule_name}", ) - timezone = value.get(TimeValues.TIMEZONE.value, "UTC") - if not isinstance(timezone, str): - raise SchemaValidationError(error_str) + ConditionsValidator._validate_timezone(timezone=value.get(TimeValues.TIMEZONE.value), rule=rule_name) - # try to see if the timezone string corresponds to any known timezone - if not tz.gettz(timezone): - raise SchemaValidationError(f"'TIMEZONE' value must represent a valid IANA timezone, rule={rule_name}") + @staticmethod + def _validate_schedule_between_time_range_key(key: str, rule_name: str): + if key != TimeKeys.CURRENT_TIME.value: + raise SchemaValidationError( + f"'condition with a 'SCHEDULE_BETWEEN_TIME_RANGE' action must have a 'CURRENT_TIME' condition key, rule={rule_name}", # noqa: E501 + ) @staticmethod - def _validate_schedule_between_time_and_datetime_ranges( - value: Any, - rule_name: str, - action_name: str, - validator: Callable[[str, str], None], - ): - error_str = f"condition with a '{action_name}' action must have a condition value type dictionary with 'START' and 'END' keys, rule={rule_name}" # noqa: E501 + def _validate_schedule_between_time_range_value(value: Dict, rule_name: str): if not isinstance(value, dict): - raise SchemaValidationError(error_str) + raise SchemaValidationError( + f"{RuleAction.SCHEDULE_BETWEEN_TIME_RANGE.value} action must have a dictionary with 'START' and 'END' keys, rule={rule_name}", # noqa: E501 + ) + + start_time = value.get(TimeValues.START.value, "") + end_time = value.get(TimeValues.END.value, "") - start_time = value.get(TimeValues.START.value) - end_time = value.get(TimeValues.END.value) - if not start_time or not end_time: - raise SchemaValidationError(error_str) if not isinstance(start_time, str) or not isinstance(end_time, str): raise SchemaValidationError(f"'START' and 'END' must be a non empty string, rule={rule_name}") - validator(start_time, rule_name) - validator(end_time, rule_name) + # Using a regex instead of strptime because it's several orders of magnitude faster + if not TIME_RANGE_PATTERN.match(start_time) or not TIME_RANGE_PATTERN.match(end_time): + raise SchemaValidationError( + f"'START' and 'END' must be a valid time format, time_format={TIME_RANGE_FORMAT}, rule={rule_name}", + ) - timezone = value.get(TimeValues.TIMEZONE.value, "UTC") - if not isinstance(timezone, str): - raise SchemaValidationError(f"'TIMEZONE' must be a string, rule={rule_name}") + ConditionsValidator._validate_timezone(timezone=value.get(TimeValues.TIMEZONE.value), rule=rule_name) - # try to see if the timezone string corresponds to any known timezone - if not tz.gettz(timezone): - raise SchemaValidationError(f"'TIMEZONE' value must represent a valid IANA timezone, rule={rule_name}") + @staticmethod + def _validate_schedule_between_datetime_range_key(key: str, rule_name: str): + if key != TimeKeys.CURRENT_DATETIME.value: + raise SchemaValidationError( + f"'condition with a 'SCHEDULE_BETWEEN_DATETIME_RANGE' action must have a 'CURRENT_DATETIME' condition key, rule={rule_name}", # noqa: E501 + ) @staticmethod - def _validate_modulo_range(value: Any, rule_name: str): + def _validate_schedule_between_datetime_range_value(value: dict, rule_name: str): + if not isinstance(value, dict): + raise SchemaValidationError( + f"{RuleAction.SCHEDULE_BETWEEN_DATETIME_RANGE.value} action must have a dictionary with 'START' and 'END' keys, rule={rule_name}", # noqa: E501 + ) + + start_time = value.get(TimeValues.START.value, "") + end_time = value.get(TimeValues.END.value, "") + + if not isinstance(start_time, str) or not isinstance(end_time, str): + raise SchemaValidationError(f"'START' and 'END' must be a non empty string, rule={rule_name}") + + ConditionsValidator._validate_datetime(start_time, rule_name) + ConditionsValidator._validate_datetime(end_time, rule_name) + ConditionsValidator._validate_timezone(timezone=value.get(TimeValues.TIMEZONE.value), rule=rule_name) + + @staticmethod + def _validate_modulo_range_value(value: dict, rule_name: str): error_str = f"condition with a 'MODULO_RANGE' action must have a condition value type dictionary with 'BASE', 'START' and 'END' keys, rule={rule_name}" # noqa: E501 if not isinstance(value, dict): raise SchemaValidationError(error_str) @@ -476,8 +480,10 @@ def _validate_modulo_range(value: Any, rule_name: str): base = value.get(ModuloRangeValues.BASE.value) start = value.get(ModuloRangeValues.START.value) end = value.get(ModuloRangeValues.END.value) + if base is None or start is None or end is None: raise SchemaValidationError(error_str) + if not isinstance(base, int) or not isinstance(start, int) or not isinstance(end, int): raise SchemaValidationError(f"'BASE', 'START' and 'END' must be integers, rule={rule_name}") @@ -485,3 +491,55 @@ def _validate_modulo_range(value: Any, rule_name: str): raise SchemaValidationError( f"condition with 'MODULO_RANGE' action must satisfy 0 <= START <= END <= BASE-1, rule={rule_name}", ) + + @staticmethod + def _validate_all_in_value_value(value: list, rule_name: str): + if not (isinstance(value, list)): + raise SchemaValidationError(f"ALL_IN_VALUE action must have a list value, rule={rule_name}") + + @staticmethod + def _validate_any_in_value_value(value: list, rule_name: str): + if not (isinstance(value, list)): + raise SchemaValidationError(f"ANY_IN_VALUE action must have a list value, rule={rule_name}") + + @staticmethod + def _validate_none_in_value_value(value: list, rule_name: str): + if not (isinstance(value, list)): + raise SchemaValidationError(f"NONE_IN_VALUE action must have a list value, rule={rule_name}") + + @staticmethod + def _validate_datetime(datetime_str: str, rule_name: str): + date = None + + # We try to parse first with timezone information in order to return the correct error messages + # when a timestamp with timezone is used. Otherwise, the user would get the first error "must be a valid + # ISO8601 time format" which is misleading + + try: + # python < 3.11 don't support the Z timezone on datetime.fromisoformat, + # so we replace any Z with the equivalent "+00:00" + # datetime.fromisoformat is orders of magnitude faster than datetime.strptime + date = datetime.fromisoformat(datetime_str.replace("Z", "+00:00")) + except Exception: + raise SchemaValidationError(f"'START' and 'END' must be a valid ISO8601 time format, rule={rule_name}") + + # we only allow timezone information to be set via the TIMEZONE field + # this way we can encode DST into the calculation. For instance, Copenhagen is + # UTC+2 during winter, and UTC+1 during summer, which would be impossible to define + # using a single ISO datetime string + if date.tzinfo is not None: + raise SchemaValidationError( + "'START' and 'END' must not include timezone information. Set the timezone using the 'TIMEZONE' " + f"field, rule={rule_name} ", + ) + + @staticmethod + def _validate_timezone(rule: str, timezone: str | None = None): + timezone = timezone or "UTC" + + if not isinstance(timezone, str): + raise SchemaValidationError(f"'TIMEZONE' must be a string, rule={str}") + + # try to see if the timezone string corresponds to any known timezone + if not tz.gettz(timezone): + raise SchemaValidationError(f"'TIMEZONE' value must represent a valid IANA timezone, rule={rule}") diff --git a/ruff.toml b/ruff.toml index 553a8c47b3d..374a183541b 100644 --- a/ruff.toml +++ b/ruff.toml @@ -1,5 +1,5 @@ # Enable rules. -select = [ +lint.select = [ "A", # flake8-builtins - https://beta.ruff.rs/docs/rules/#flake8-builtins-a "B", # flake8-bugbear-b - https://beta.ruff.rs/docs/rules/#flake8-bugbear-b "C4", # flake8-comprehensions - https://beta.ruff.rs/docs/rules/#flake8-comprehensions-c4 @@ -28,7 +28,7 @@ select = [ ] # Ignore specific rules -ignore = [ +lint.ignore = [ "W291", # https://beta.ruff.rs/docs/rules/trailing-whitespace/ "PLR0913", # https://beta.ruff.rs/docs/rules/too-many-arguments/ "PLR2004", #https://beta.ruff.rs/docs/rules/magic-value-comparison/ @@ -60,28 +60,28 @@ line-length = 120 fix = true -fixable = ["I", "COM812", "W"] +lint.fixable = ["I", "COM812", "W"] # See: https://github.com/astral-sh/ruff/issues/128 -typing-modules = [ +lint.typing-modules = [ "aws_lambda_powertools.utilities.parser.types", "aws_lambda_powertools.shared.types", ] -[mccabe] +[lint.mccabe] # Maximum cyclomatic complexity max-complexity = 15 -[pylint] +[lint.pylint] # Maximum number of nested blocks max-branches = 15 # Maximum number of if statements in a function max-statements = 70 -[isort] +[lint.isort] split-on-trailing-comma = true -[per-file-ignores] +[lint.per-file-ignores] # Ignore specific rules for specific files "tests/e2e/utils/data_builder/__init__.py" = ["F401"] "tests/e2e/utils/data_fetcher/__init__.py" = ["F401"] diff --git a/tests/functional/feature_flags/test_feature_flags.py b/tests/functional/feature_flags/test_feature_flags.py index 12adaa4525b..cc6aa60aaac 100644 --- a/tests/functional/feature_flags/test_feature_flags.py +++ b/tests/functional/feature_flags/test_feature_flags.py @@ -1410,3 +1410,260 @@ def test_get_all_enabled_features_non_boolean_truthy_defaults(mocker, config): feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config) enabled_list: List[str] = feature_flags.get_enabled_features(context={"tenant_id": "6", "username": "a"}) assert enabled_list == expected_value + + +def test_flags_any_in_value_match(mocker, config): + expected_value = True + mocked_app_config_schema = { + "my_feature": { + "default": False, + "rules": { + "tenant_id is in allowed list": { + "when_match": expected_value, + "conditions": [ + { + "action": RuleAction.ANY_IN_VALUE.value, + "key": "tenant_id", + "value": ["Łukasz", "Gerald", "Leandro", "Heitor"], + }, + ], + }, + }, + }, + } + + feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config) + toggle = feature_flags.evaluate( + name="my_feature", + context={"tenant_id": ["Gerald"]}, + default=False, + ) + assert toggle == expected_value + + +def test_flags_any_in_value_no_match(mocker, config): + expected_value = False + mocked_app_config_schema = { + "my_feature": { + "default": False, + "rules": { + "tenant_id is in allowed list": { + "when_match": expected_value, + "conditions": [ + { + "action": RuleAction.ANY_IN_VALUE.value, + "key": "tenant_id", + "value": ["Łukasz", "Gerald", "Leandro", "Heitor"], + }, + ], + }, + }, + }, + } + + feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config) + toggle = feature_flags.evaluate( + name="my_feature", + context={"tenant_id": ["Simon"]}, + default=False, + ) + assert toggle == expected_value + + +def test_flags_all_in_value_match(mocker, config): + expected_value = True + mocked_app_config_schema = { + "my_feature": { + "default": False, + "rules": { + "tenant_id is in allowed list": { + "when_match": expected_value, + "conditions": [ + { + "action": RuleAction.ALL_IN_VALUE.value, + "key": "tenant_id", + "value": ["Łukasz", "Gerald", "Leandro", "Heitor"], + }, + ], + }, + }, + }, + } + + feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config) + toggle = feature_flags.evaluate( + name="my_feature", + context={"tenant_id": ["Gerald"]}, + default=False, + ) + + assert toggle == expected_value + + +def test_flags_all_in_value_no_match(mocker, config): + expected_value = False + mocked_app_config_schema = { + "my_feature": { + "default": False, + "rules": { + "tenant_id is in allowed list": { + "when_match": expected_value, + "conditions": [ + { + "action": RuleAction.ALL_IN_VALUE.value, + "key": "tenant_id", + "value": ["Łukasz", "Gerald", "Leandro", "Heitor"], + }, + ], + }, + }, + }, + } + + feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config) + toggle = feature_flags.evaluate( + name="my_feature", + context={"tenant_id": ["Gerald", "Simon"]}, + default=False, + ) + + assert toggle == expected_value + + +def test_flags_none_in_value_match(mocker, config): + expected_value = True + mocked_app_config_schema = { + "my_feature": { + "default": False, + "rules": { + "tenant_id is in allowed list": { + "when_match": expected_value, + "conditions": [ + { + "action": RuleAction.NONE_IN_VALUE.value, + "key": "tenant_id", + "value": ["Łukasz", "Gerald", "Leandro", "Heitor"], + }, + ], + }, + }, + }, + } + + feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config) + toggle = feature_flags.evaluate( + name="my_feature", + context={"tenant_id": ["Rubao"]}, + default=False, + ) + + assert toggle == expected_value + + +def test_flags_none_in_value_no_match(mocker, config): + expected_value = False + mocked_app_config_schema = { + "my_feature": { + "default": False, + "rules": { + "tenant_id is in allowed list": { + "when_match": expected_value, + "conditions": [ + { + "action": RuleAction.NONE_IN_VALUE.value, + "key": "tenant_id", + "value": ["Łukasz", "Gerald", "Leandro", "Heitor"], + }, + ], + }, + }, + }, + } + + feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config) + toggle = feature_flags.evaluate( + name="my_feature", + context={"tenant_id": ["Heitor"]}, + default=False, + ) + + assert toggle == expected_value + + +@pytest.mark.parametrize( + "intersection_action", + [ + RuleAction.ALL_IN_VALUE.value, + RuleAction.ANY_IN_VALUE.value, + RuleAction.NONE_IN_VALUE.value, + ], +) +def test_intersection_non_list_value(mocker, config, intersection_action): + # GIVEN a schema with list intersection action + expected_value = False + mocked_app_config_schema = { + "my_feature": { + "default": False, + "rules": { + "tenant_id is in allowed list": { + "when_match": expected_value, + "conditions": [ + { + "action": intersection_action, + "key": "tenant_id", + "value": ["Łukasz", "Gerald", "Leandro", "Heitor"], + }, + ], + }, + }, + }, + } + + feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config) + + # WHEN a context value isn't a list + toggle = feature_flags.evaluate( + name="my_feature", + context={"tenant_id": "not a list value"}, + default=False, + ) + + # THEN TypeError should be swallowed and use default value + assert toggle == expected_value + + +def test_exception_handler(mocker, config): + # GIVEN a schema with list intersection action + expected_value = False + mocked_app_config_schema = { + "my_feature": { + "default": False, + "rules": { + "tenant_id is in allowed list": { + "when_match": expected_value, + "conditions": [ + { + "action": RuleAction.ANY_IN_VALUE.value, + "key": "tenant_id", + "value": ["Łukasz", "Gerald", "Leandro", "Heitor"], + }, + ], + }, + }, + }, + } + + feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config) + + @feature_flags.validation_exception_handler(ValueError) + def catch_exception(exc): + raise TypeError("re-raised") + + # WHEN a context value isn't a list + # THEN exception handler should be able to intercept and raise, instead of returning `False` + with pytest.raises(TypeError): + feature_flags.evaluate( + name="my_feature", + context={"tenant_id": "not a list value"}, + default=False, + ) diff --git a/tests/functional/feature_flags/test_schema_validation.py b/tests/functional/feature_flags/test_schema_validation.py index 654cfbcab40..45b4c7dbeda 100644 --- a/tests/functional/feature_flags/test_schema_validation.py +++ b/tests/functional/feature_flags/test_schema_validation.py @@ -1,8 +1,8 @@ -import logging import re -import pytest # noqa: F401 +import pytest +from aws_lambda_powertools.logging.logger import Logger # noqa: F401 from aws_lambda_powertools.utilities.feature_flags.exceptions import ( SchemaValidationError, ) @@ -24,8 +24,6 @@ TimeValues, ) -logger = logging.getLogger(__name__) - EMPTY_SCHEMA = {"": ""} @@ -441,7 +439,7 @@ def test_validate_time_condition_between_time_range_invalid_condition_value(): # THEN raise SchemaValidationError with pytest.raises( SchemaValidationError, - match=f"condition with a 'SCHEDULE_BETWEEN_TIME_RANGE' action must have a condition value type dictionary with 'START' and 'END' keys, rule={rule_name}", # noqa: E501 + match=f"SCHEDULE_BETWEEN_TIME_RANGE action must have a dictionary with 'START' and 'END' keys, rule={rule_name}", # noqa: E501 ): ConditionsValidator.validate_condition_value(condition=condition, rule_name=rule_name) @@ -460,7 +458,7 @@ def test_validate_time_condition_between_time_range_invalid_condition_value_no_s # THEN raise SchemaValidationError with pytest.raises( SchemaValidationError, - match=f"condition with a 'SCHEDULE_BETWEEN_TIME_RANGE' action must have a condition value type dictionary with 'START' and 'END' keys, rule={rule_name}", # noqa: E501 + match="'START' and 'END' must be a valid time format", ): ConditionsValidator.validate_condition_value(condition=condition, rule_name=rule_name) @@ -477,10 +475,7 @@ def test_validate_time_condition_between_time_range_invalid_condition_value_no_e # WHEN calling validate_condition # THEN raise SchemaValidationError - with pytest.raises( - SchemaValidationError, - match=f"condition with a 'SCHEDULE_BETWEEN_TIME_RANGE' action must have a condition value type dictionary with 'START' and 'END' keys, rule={rule_name}", # noqa: E501 - ): + with pytest.raises(SchemaValidationError, match="'START' and 'END' must be a valid time format"): ConditionsValidator.validate_condition_value(condition=condition, rule_name=rule_name) @@ -649,7 +644,7 @@ def test_a_validate_time_condition_between_datetime_range_invalid_condition_valu # THEN raise SchemaValidationError with pytest.raises( SchemaValidationError, - match=f"condition with a 'SCHEDULE_BETWEEN_DATETIME_RANGE' action must have a condition value type dictionary with 'START' and 'END' keys, rule={rule_name}", # noqa: E501 + match=f"SCHEDULE_BETWEEN_DATETIME_RANGE action must have a dictionary with 'START' and 'END' keys, rule={rule_name}", # noqa: E501 ): ConditionsValidator.validate_condition_value(condition=condition, rule_name=rule_name) @@ -668,7 +663,7 @@ def test_validate_time_condition_between_datetime_range_invalid_condition_value_ # THEN raise SchemaValidationError with pytest.raises( SchemaValidationError, - match=f"condition with a 'SCHEDULE_BETWEEN_DATETIME_RANGE' action must have a condition value type dictionary with 'START' and 'END' keys, rule={rule_name}", # noqa: E501 + match=f"'START' and 'END' must be a valid ISO8601 time format, rule={rule_name}", ): ConditionsValidator.validate_condition_value(condition=condition, rule_name=rule_name) @@ -687,7 +682,7 @@ def test_validate_time_condition_between_datetime_range_invalid_condition_value_ # THEN raise SchemaValidationError with pytest.raises( SchemaValidationError, - match=f"condition with a 'SCHEDULE_BETWEEN_DATETIME_RANGE' action must have a condition value type dictionary with 'START' and 'END' keys, rule={rule_name}", # noqa: E501 + match="'START' and 'END' must not include timezone information.*", ): ConditionsValidator.validate_condition_value(condition=condition, rule_name=rule_name) @@ -1032,3 +1027,48 @@ def test_validate_modulo_range_condition_valid(): # WHEN calling validate_condition # THEN nothing is raised ConditionsValidator.validate_condition_value(condition=condition, rule_name="dummy") + + +def test_validate_any_in_value_condition_invalid_value(): + # GIVEN a schema with a ANY_IN_VALUE action with non-list value + condition = { + CONDITION_ACTION: RuleAction.ANY_IN_VALUE.value, + CONDITION_VALUE: "Gerald", + } + + rule_name = "non-list value for ANY_IN_VALUE" + + # WHEN calling validate_condition + # THEN raise SchemaValidationError + with pytest.raises(SchemaValidationError, match="ANY_IN_VALUE action must have a list"): + ConditionsValidator.validate_condition_value(condition=condition, rule_name=rule_name) + + +def test_validate_all_in_value_condition_invalid_value(): + # GIVEN a schema with a ANY_IN_VALUE action with non-list value + condition = { + CONDITION_ACTION: RuleAction.ALL_IN_VALUE.value, + CONDITION_VALUE: "Leandro", + } + + rule_name = "non-list value for ALL_IN_VALUE" + + # WHEN calling validate_condition + # THEN raise SchemaValidationError + with pytest.raises(SchemaValidationError, match="ALL_IN_VALUE action must have a list"): + ConditionsValidator.validate_condition_value(condition=condition, rule_name=rule_name) + + +def test_validate_none_in_value_condition_invalid_value(): + # GIVEN a schema with a ANY_IN_VALUE action with non-list value + condition = { + CONDITION_ACTION: RuleAction.NONE_IN_VALUE.value, + CONDITION_VALUE: "Heitor", + } + + rule_name = "non-list value for NONE_IN_VALUE" + + # WHEN calling validate_condition + # THEN raise SchemaValidationError + with pytest.raises(SchemaValidationError, match="NONE_IN_VALUE action must have a list"): + ConditionsValidator.validate_condition_value(condition=condition, rule_name=rule_name)