From dc6f809c39e364a02291ff05564f8fb896755954 Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Wed, 30 Jun 2021 09:48:46 +0300 Subject: [PATCH 01/12] feat: Feature toggles rule engine --- .../utilities/feature_toggles/__init__.py | 9 + .../feature_toggles/configuration_store.py | 207 ++++++++++ tests/functional/feature_toggles/__init__.py | 0 .../feature_toggles/test_feature_toggles.py | 374 ++++++++++++++++++ 4 files changed, 590 insertions(+) create mode 100644 aws_lambda_powertools/utilities/feature_toggles/__init__.py create mode 100644 aws_lambda_powertools/utilities/feature_toggles/configuration_store.py create mode 100644 tests/functional/feature_toggles/__init__.py create mode 100644 tests/functional/feature_toggles/test_feature_toggles.py diff --git a/aws_lambda_powertools/utilities/feature_toggles/__init__.py b/aws_lambda_powertools/utilities/feature_toggles/__init__.py new file mode 100644 index 0000000000..8a8ba28e06 --- /dev/null +++ b/aws_lambda_powertools/utilities/feature_toggles/__init__.py @@ -0,0 +1,9 @@ +"""Advanced feature toggles utility +""" +from .configuration_store import ACTION, ConfigurationException, ConfigurationStore + +__all__ = [ + "ConfigurationException", + "ConfigurationStore", + "ACTION", +] diff --git a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py new file mode 100644 index 0000000000..6f33a5b417 --- /dev/null +++ b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py @@ -0,0 +1,207 @@ +# pylint: disable=no-name-in-module,line-too-long +from enum import Enum +from typing import Any, Dict, List, Optional + +from botocore.config import Config + +from aws_lambda_powertools.utilities.parameters import AppConfigProvider, GetParameterError, TransformParameterError + +TRANSFORM_TYPE = "json" +FEATURES_KEY = "features" +RULES_KEY = "rules" +DEFAULT_VAL_KEY = "default_value" +RESTRICTIONS_KEY = "restrictions" +RULE_NAME_KEY = "name" +RULE_DEFAULT_VALUE = "default_value" +RESTRICTION_KEY = "key" +RESTRICTION_VALUE = "value" +RESTRICTION_ACTION = "action" + + +class ACTION(str, Enum): + EQUALS = "EQUALS" + STARTSWITH = "STARTSWITH" + ENDSWITH = "ENDSWITH" + CONTAINS = "CONTAINS" + + +class ConfigurationException(Exception): + pass + + +class ConfigurationStore: + def __init__( + self, environment: str, service: str, conf_name: str, cache_seconds: int, region_name: Optional[str] = None + ): + """constructor + + Args: + environment (str): what appconfig environment to use 'dev/test' etc. + service (str): what service name to use from the supplied environment + conf_name (str): what configuration to take from the environment & service combination + cache_seconds (int): cache expiration time, how often to call AppConfig to fetch latest configuration + region_name (str): aws region where the configuration resides in + """ + self._cache_seconds = cache_seconds + self._conf_name = conf_name + config = None + if region_name is not None: + config = Config(region_name=region_name) + self._conf_store = AppConfigProvider(environment=environment, application=service, config=config) + + def _validate_json_schema(self, schema: str) -> bool: + ## todo + return True + + def _match_by_action(self, action: str, restriction_value: Any, context_value: Any) -> bool: + mapping_by_action = { + ACTION.EQUALS.value: lambda a, b: a == b, + ACTION.STARTSWITH.value: lambda a, b: a.startswith(b), + ACTION.ENDSWITH.value: lambda a, b: a.endswith(b), + ACTION.CONTAINS.value: lambda a, b: a in b, + } + + try: + func = mapping_by_action.get(action, lambda a, b: False) + return func(context_value, restriction_value) + except Exception: + return False + + def _handle_rules( + self, + logger: object, + feature_name: str, + rules_context: Dict[str, Any], + default_value: bool, + rules: List[Dict[str, Any]], + ) -> bool: + for rule in rules: + rule_name = rule.get(RULE_NAME_KEY, "") + rule_default_value = rule.get(RULE_DEFAULT_VALUE) + is_match = True + restrictions: Dict[str, str] = rule.get(RESTRICTIONS_KEY) + if restrictions is None: + error_str = f"invalid rule schema detected in rule {rule_name}, missing restrictions list" + logger.error(error_str) + raise ConfigurationException(error_str) + + for restriction in restrictions: + context_value = rules_context.get(restriction.get(RESTRICTION_KEY, "")) + if not context_value: + logger.debug( + "rule did not not match key", + rule_name=rule_name, + default_value=rule_default_value, + feature_name=feature_name, + ) + is_match = False # rule doesn't match, continue to next one + break + elif not self._match_by_action( + restriction.get(RESTRICTION_ACTION), restriction.get(RESTRICTION_VALUE), context_value + ): + logger.debug( + "rule did not match action", + rule_name=rule_name, + default_value=rule_default_value, + feature_name=feature_name, + ) + is_match = False # rules doesn't match restriction + break + # if we got here, all restrictions match + if is_match: + logger.debug( + "rule matched", rule_name=rule_name, default_value=rule_default_value, feature_name=feature_name + ) + return rule_default_value + # no rule matched, return default value of feature + logger.debug( + "no rule matched, returning default value of feature", + default_value=default_value, + feature_name=feature_name, + ) + return default_value + + def get_configuration(self, logger: object) -> Dict[str, Any]: + """Get configuration string from AWs AppConfig and returned the parsed JSON dictionary + + Args: + logger (object): logger class to use. must have a debug, info, error, warning and exception functions that + receive a message and kwargs + + Raises: + ConfigurationException: Any validation error or appconfig error that can occur + + Returns: + Dict[str, Any]: parsed JSON dictionary + """ + try: + schema = self._conf_store.get( + name=self._conf_name, + transform=TRANSFORM_TYPE, + max_age=self._cache_seconds, + ) # parse result conf as JSON, keep in cache for self.max_age seconds + except (GetParameterError, TransformParameterError) as exc: + error_str = f"unable to get AWS AppConfig configuration file, exception={str(exc)}" + logger.error(error_str) + raise ConfigurationException(error_str) + # validate schema + if not self._validate_json_schema(schema): + error_str = "AWS AppConfig schema failed validation" + logger.error(error_str) + raise ConfigurationException(error_str) + return schema + + def get_feature_toggle( + self, logger: object, feature_name: str, rules_context: Dict[str, Any], default_value: bool + ) -> bool: + """get a feature toggle boolean value. Value is calculated according to a set of rules and conditions. + see below for explanation. + + Args: + logger (object): logger class to use. must have a debug, info, error, warning and exception functions + that receive a message and kwargs + feature_name (str): feature name that you wish to fetch + rules_context (Dict[str, Any]): dict of attributes that you would like to match the rules + against, can be {'tenant_id: 'X', 'username':' 'Y', 'region': 'Z'} etc. + default_value (bool): this will be the returned value in case the feature toggle doesn't exist in the schema + or there has been an error while fetching the configuration from appconfig + + Returns: + bool: calculated feature toggle value. several possibilities: + 1. if the feature doesn't appear in the schema or there has been an error fetching the + configuration -> error log would appear and default_value is returned + 2. feature exists and has no rules or no rules have matched -> return default_value of + the defined feature + 3. feature exists and a rule matches -> default_value of rule is returned + """ + try: + toggles_dict: Dict[str, Any] = self.get_configuration(logger=logger) + except ConfigurationException: + logger.warning("unable to get feature toggles JSON, returning provided default value") + return default_value + + feature: Dict[str, Dict] = toggles_dict.get(FEATURES_KEY, {}).get(feature_name, None) + if feature is None: + logger.warning( + "feature does not appear in configuration, using provided default value", + feature_name=feature_name, + default_value=default_value, + ) + return default_value + + rules_list = feature.get(RULES_KEY, []) + def_val = feature.get(DEFAULT_VAL_KEY) + if def_val is None: + error_str = f"invalid feature schema, missing default value, feature={feature_name}" + logger.error(error_str) + raise ConfigurationException(error_str) + + if not rules_list: + # not rules but has a value + logger.debug( + "no rules found, returning feature default value", feature_name=feature_name, default_value=def_val + ) + return def_val + # look for first rule match + logger.debug("looking for rule match", feature_name=feature_name, feature_default_value=def_val) + return self._handle_rules(logger, feature_name, rules_context, def_val, rules_list) diff --git a/tests/functional/feature_toggles/__init__.py b/tests/functional/feature_toggles/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/functional/feature_toggles/test_feature_toggles.py b/tests/functional/feature_toggles/test_feature_toggles.py new file mode 100644 index 0000000000..01ebae7bc6 --- /dev/null +++ b/tests/functional/feature_toggles/test_feature_toggles.py @@ -0,0 +1,374 @@ +from typing import Dict + +import pytest # noqa: F401 + +from aws_lambda_powertools.utilities.feature_toggles.configuration_store import ACTION, ConfigurationStore + + +class MockLogger: + def debug(self, message: str, **kwargs) -> None: + pass + + def info(self, message: str, **kwargs) -> None: + pass + + def warning(self, message: str, **kwargs) -> None: + pass + + def error(self, message: str, **kwargs) -> None: + pass + + def exception(self, message: str, **kwargs) -> None: + pass + + +def init_configuration_store(mocker, mock_schema: Dict) -> ConfigurationStore: + mocked_get_conf = mocker.patch("aws_lambda_powertools.utilities.parameters.AppConfigProvider.get") + mocked_get_conf.return_value = mock_schema + conf_store: ConfigurationStore = ConfigurationStore( + environment="test_env", + service="test_app", + conf_name="test_conf_name", + cache_seconds=600, + region_name="us-east-1", + ) + return conf_store + + +# this test checks that we get correct value of feature that exists in the schema. +# we also don't send an empty rules_context dict in this case +def test_toggles_no_restrictions(mocker): + expected_value = "True" + mocked_app_config_schema = { + "log_level": "DEBUG", + "features": { + "my_feature": { + "default_value": expected_value, + "rules": [ + { + "name": "tenant id equals 345345435", + "default_value": "False", + "restrictions": [ + { + "action": ACTION.EQUALS.value, + "key": "tenant_id", + "value": "345345435", + } + ], + }, + ], + } + }, + } + + logger = MockLogger() + conf_store = init_configuration_store(mocker, mocked_app_config_schema) + toggle = conf_store.get_feature_toggle( + logger=logger, feature_name="my_feature", rules_context={}, default_value=False + ) + assert str(toggle) == expected_value + + +# this test checks that if you try to get a feature that doesn't exist in the schema, +# you get the default value of False that was sent to the get_feature_toggle API +def test_toggles_no_restrictions_feature_does_not_exist(mocker): + expected_value = False + mocked_app_config_schema = {"log_level": "DEBUG", "features": {"my_fake_feature": {"default_value": "True"}}} + + logger = MockLogger() + conf_store = init_configuration_store(mocker, mocked_app_config_schema) + toggle = conf_store.get_feature_toggle( + logger=logger, feature_name="my_feature", rules_context={}, default_value=expected_value + ) + assert toggle == expected_value + + +# check that feature match works when they are no rules and we send rules_context. +# default value is False but the feature has a True default_value. +def test_toggles_no_rules(mocker): + expected_value = "True" + mocked_app_config_schema = {"log_level": "DEBUG", "features": {"my_feature": {"default_value": expected_value}}} + logger = MockLogger() + conf_store = init_configuration_store(mocker, mocked_app_config_schema) + toggle = conf_store.get_feature_toggle( + logger=logger, feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, default_value=False + ) + assert str(toggle) == expected_value + + +# check a case where the feature exists but the rule doesn't match so we revert to the default value of the feature +def test_toggles_restrictions_no_match(mocker): + expected_value = "True" + mocked_app_config_schema = { + "log_level": "DEBUG", + "features": { + "my_feature": { + "default_value": expected_value, + "rules": [ + { + "name": "tenant id equals 345345435", + "default_value": "False", + "restrictions": [ + { + "action": ACTION.EQUALS.value, + "key": "tenant_id", + "value": "345345435", + } + ], + }, + ], + } + }, + } + logger = MockLogger() + conf_store = init_configuration_store(mocker, mocked_app_config_schema) + toggle = conf_store.get_feature_toggle( + logger=logger, + feature_name="my_feature", + rules_context={"tenant_id": "6", "username": "a"}, # rule will not match + default_value=False, + ) + assert str(toggle) == expected_value + + +# check that a rule can match when it has multiple restrictions, see rule name for further explanation +def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker): + expected_value = "False" + tenant_id_val = "6" + username_val = "a" + mocked_app_config_schema = { + "log_level": "DEBUG", + "features": { + "my_feature": { + "default_value": "True", + "rules": [ + { + "name": "tenant id equals 6 and username is a", + "default_value": expected_value, + "restrictions": [ + { + "action": ACTION.EQUALS.value, # this rule will match, it has multiple restrictions + "key": "tenant_id", + "value": tenant_id_val, + }, + { + "action": ACTION.EQUALS.value, + "key": "username", + "value": username_val, + }, + ], + }, + ], + } + }, + } + logger = MockLogger() + conf_store = init_configuration_store(mocker, mocked_app_config_schema) + toggle = conf_store.get_feature_toggle( + logger=logger, + feature_name="my_feature", + rules_context={ + "tenant_id": tenant_id_val, + "username": username_val, + }, + default_value=True, + ) + assert str(toggle) == expected_value + + +# check a case when rule doesn't match and it has multiple restrictions, +# different tenant id causes the rule to not match. +# default value of the feature in this case is True +def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker): + expected_val = "True" + mocked_app_config_schema = { + "log_level": "DEBUG", + "features": { + "my_feature": { + "default_value": expected_val, + "rules": [ + { + "name": "tenant id equals 645654 and username is a", # rule will not match + "default_value": "False", + "restrictions": [ + { + "action": ACTION.EQUALS.value, + "key": "tenant_id", + "value": "645654", + }, + { + "action": ACTION.EQUALS.value, + "key": "username", + "value": "a", + }, + ], + }, + ], + } + }, + } + logger = MockLogger() + conf_store = init_configuration_store(mocker, mocked_app_config_schema) + toggle = conf_store.get_feature_toggle( + logger=logger, feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, default_value=False + ) + assert str(toggle) == expected_val + + +# check rule match for multiple of action types +def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multiple_restrictions(mocker): + expected_value_first_check = "True" + expected_value_second_check = "False" + expected_value_third_check = "False" + expected_value_fourth_case = False + mocked_app_config_schema = { + "log_level": "DEBUG", + "features": { + "my_feature": { + "default_value": expected_value_third_check, + "rules": [ + { + "name": "tenant id equals 6 and username startswith a", + "default_value": expected_value_first_check, + "restrictions": [ + { + "action": ACTION.EQUALS.value, + "key": "tenant_id", + "value": "6", + }, + { + "action": ACTION.STARTSWITH.value, + "key": "username", + "value": "a", + }, + ], + }, + { + "name": "tenant id equals 4446 and username startswith a and endswith z", + "default_value": expected_value_second_check, + "restrictions": [ + { + "action": ACTION.EQUALS.value, + "key": "tenant_id", + "value": "4446", + }, + { + "action": ACTION.STARTSWITH.value, + "key": "username", + "value": "a", + }, + { + "action": ACTION.ENDSWITH.value, + "key": "username", + "value": "z", + }, + ], + }, + ], + } + }, + } + + logger = MockLogger() + conf_store = init_configuration_store(mocker, mocked_app_config_schema) + # match first rule + toggle = conf_store.get_feature_toggle( + logger=logger, + feature_name="my_feature", + rules_context={"tenant_id": "6", "username": "abcd"}, + default_value=False, + ) + assert str(toggle) == expected_value_first_check + # match second rule + toggle = conf_store.get_feature_toggle( + logger=logger, + feature_name="my_feature", + rules_context={"tenant_id": "4446", "username": "az"}, + default_value=False, + ) + assert str(toggle) == expected_value_second_check + # match no rule + toggle = conf_store.get_feature_toggle( + logger=logger, + feature_name="my_feature", + rules_context={"tenant_id": "11114446", "username": "ab"}, + default_value=False, + ) + assert str(toggle) == expected_value_third_check + # feature doesn't exist + toggle = conf_store.get_feature_toggle( + logger=logger, + feature_name="my_fake_feature", + rules_context={"tenant_id": "11114446", "username": "ab"}, + default_value=expected_value_fourth_case, + ) + assert toggle == expected_value_fourth_case + + +# check a case where the feature exists but the rule doesn't match so we revert to the default value of the feature +def test_toggles_match_rule_with_contains_action(mocker): + expected_value = True + mocked_app_config_schema = { + "log_level": "DEBUG", + "features": { + "my_feature": { + "default_value": expected_value, + "rules": [ + { + "name": "tenant id equals 345345435", + "default_value": True, + "restrictions": [ + { + "action": ACTION.CONTAINS.value, + "key": "tenant_id", + "value": ["6", "2"], + } + ], + }, + ], + } + }, + } + logger = MockLogger() + conf_store = init_configuration_store(mocker, mocked_app_config_schema) + toggle = conf_store.get_feature_toggle( + logger=logger, + feature_name="my_feature", + rules_context={"tenant_id": "6", "username": "a"}, # rule will match + default_value=False, + ) + assert toggle == expected_value + + +def test_toggles_no_match_rule_with_contains_action(mocker): + expected_value = False + mocked_app_config_schema = { + "log_level": "DEBUG", + "features": { + "my_feature": { + "default_value": expected_value, + "rules": [ + { + "name": "tenant id equals 345345435", + "default_value": True, + "restrictions": [ + { + "action": ACTION.CONTAINS.value, + "key": "tenant_id", + "value": ["8", "2"], + } + ], + }, + ], + } + }, + } + logger = MockLogger() + conf_store = init_configuration_store(mocker, mocked_app_config_schema) + toggle = conf_store.get_feature_toggle( + logger=logger, + feature_name="my_feature", + rules_context={"tenant_id": "6", "username": "a"}, # rule will not match + default_value=False, + ) + assert toggle == expected_value From 5b5dd3ec2ba0e5adcce577a4ad084b13bb0d7fb9 Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Wed, 30 Jun 2021 20:59:12 +0300 Subject: [PATCH 02/12] cr fixes --- .../utilities/feature_toggles/__init__.py | 3 +- .../feature_toggles/configuration_store.py | 83 +++++++------------ .../utilities/feature_toggles/exceptions.py | 2 + .../feature_toggles/test_feature_toggles.py | 49 ++--------- 4 files changed, 37 insertions(+), 100 deletions(-) create mode 100644 aws_lambda_powertools/utilities/feature_toggles/exceptions.py diff --git a/aws_lambda_powertools/utilities/feature_toggles/__init__.py b/aws_lambda_powertools/utilities/feature_toggles/__init__.py index 8a8ba28e06..36789f6362 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/__init__.py +++ b/aws_lambda_powertools/utilities/feature_toggles/__init__.py @@ -1,6 +1,7 @@ """Advanced feature toggles utility """ -from .configuration_store import ACTION, ConfigurationException, ConfigurationStore +from .configuration_store import ACTION, ConfigurationStore +from .exceptions import ConfigurationException __all__ = [ "ConfigurationException", diff --git a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py index 6f33a5b417..04700de1af 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py +++ b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py @@ -1,4 +1,5 @@ # pylint: disable=no-name-in-module,line-too-long +import logging from enum import Enum from typing import Any, Dict, List, Optional @@ -6,6 +7,8 @@ from aws_lambda_powertools.utilities.parameters import AppConfigProvider, GetParameterError, TransformParameterError +from .exceptions import ConfigurationException + TRANSFORM_TYPE = "json" FEATURES_KEY = "features" RULES_KEY = "rules" @@ -25,13 +28,12 @@ class ACTION(str, Enum): CONTAINS = "CONTAINS" -class ConfigurationException(Exception): - pass +logger = logging.getLogger(__name__) class ConfigurationStore: def __init__( - self, environment: str, service: str, conf_name: str, cache_seconds: int, region_name: Optional[str] = None + self, environment: str, service: str, conf_name: str, cache_seconds: int, config: Optional[Config] = None ): """constructor @@ -40,13 +42,11 @@ def __init__( service (str): what service name to use from the supplied environment conf_name (str): what configuration to take from the environment & service combination cache_seconds (int): cache expiration time, how often to call AppConfig to fetch latest configuration - region_name (str): aws region where the configuration resides in """ self._cache_seconds = cache_seconds + self.logger = logger self._conf_name = conf_name - config = None - if region_name is not None: - config = Config(region_name=region_name) + config = config or Config() self._conf_store = AppConfigProvider(environment=environment, application=service, config=config) def _validate_json_schema(self, schema: str) -> bool: @@ -54,6 +54,8 @@ def _validate_json_schema(self, schema: str) -> bool: return True def _match_by_action(self, action: str, restriction_value: Any, context_value: Any) -> bool: + if not context_value: + return False mapping_by_action = { ACTION.EQUALS.value: lambda a, b: a == b, ACTION.STARTSWITH.value: lambda a, b: a.startswith(b), @@ -69,7 +71,7 @@ def _match_by_action(self, action: str, restriction_value: Any, context_value: A def _handle_rules( self, - logger: object, + *, feature_name: str, rules_context: Dict[str, Any], default_value: bool, @@ -80,54 +82,32 @@ def _handle_rules( rule_default_value = rule.get(RULE_DEFAULT_VALUE) is_match = True restrictions: Dict[str, str] = rule.get(RESTRICTIONS_KEY) - if restrictions is None: - error_str = f"invalid rule schema detected in rule {rule_name}, missing restrictions list" - logger.error(error_str) - raise ConfigurationException(error_str) for restriction in restrictions: - context_value = rules_context.get(restriction.get(RESTRICTION_KEY, "")) - if not context_value: - logger.debug( - "rule did not not match key", - rule_name=rule_name, - default_value=rule_default_value, - feature_name=feature_name, - ) - is_match = False # rule doesn't match, continue to next one - break - elif not self._match_by_action( + context_value = rules_context.get(restriction.get(RESTRICTION_KEY, ""), "") + if not self._match_by_action( restriction.get(RESTRICTION_ACTION), restriction.get(RESTRICTION_VALUE), context_value ): logger.debug( - "rule did not match action", - rule_name=rule_name, - default_value=rule_default_value, - feature_name=feature_name, + f"rule did not match action, rule_name={rule_name}, default_value={rule_default_value}, feature_name={feature_name}, context_value={str(context_value)}" # noqa: E501 ) is_match = False # rules doesn't match restriction break # if we got here, all restrictions match if is_match: logger.debug( - "rule matched", rule_name=rule_name, default_value=rule_default_value, feature_name=feature_name + f"rule matched, rule_name={rule_name}, default_value={rule_default_value}, feature_name={feature_name}" # noqa: E501 ) return rule_default_value # no rule matched, return default value of feature logger.debug( - "no rule matched, returning default value of feature", - default_value=default_value, - feature_name=feature_name, + f"no rule matched, returning default value of feature, default_value={default_value}, feature_name={feature_name}" # noqa: E501 ) return default_value - def get_configuration(self, logger: object) -> Dict[str, Any]: + def get_configuration(self) -> Dict[str, Any]: """Get configuration string from AWs AppConfig and returned the parsed JSON dictionary - Args: - logger (object): logger class to use. must have a debug, info, error, warning and exception functions that - receive a message and kwargs - Raises: ConfigurationException: Any validation error or appconfig error that can occur @@ -151,15 +131,11 @@ def get_configuration(self, logger: object) -> Dict[str, Any]: raise ConfigurationException(error_str) return schema - def get_feature_toggle( - self, logger: object, feature_name: str, rules_context: Dict[str, Any], default_value: bool - ) -> bool: + def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any], default_value: bool) -> bool: """get a feature toggle boolean value. Value is calculated according to a set of rules and conditions. see below for explanation. Args: - logger (object): logger class to use. must have a debug, info, error, warning and exception functions - that receive a message and kwargs feature_name (str): feature name that you wish to fetch rules_context (Dict[str, Any]): dict of attributes that you would like to match the rules against, can be {'tenant_id: 'X', 'username':' 'Y', 'region': 'Z'} etc. @@ -175,7 +151,7 @@ def get_feature_toggle( 3. feature exists and a rule matches -> default_value of rule is returned """ try: - toggles_dict: Dict[str, Any] = self.get_configuration(logger=logger) + toggles_dict: Dict[str, Any] = self.get_configuration() except ConfigurationException: logger.warning("unable to get feature toggles JSON, returning provided default value") return default_value @@ -183,25 +159,22 @@ def get_feature_toggle( feature: Dict[str, Dict] = toggles_dict.get(FEATURES_KEY, {}).get(feature_name, None) if feature is None: logger.warning( - "feature does not appear in configuration, using provided default value", - feature_name=feature_name, - default_value=default_value, + f"feature does not appear in configuration, using provided default value, feature_name={feature_name}, default_value={default_value}" # noqa: E501 ) return default_value rules_list = feature.get(RULES_KEY, []) - def_val = feature.get(DEFAULT_VAL_KEY) - if def_val is None: - error_str = f"invalid feature schema, missing default value, feature={feature_name}" - logger.error(error_str) - raise ConfigurationException(error_str) - + default_value = feature.get(DEFAULT_VAL_KEY) if not rules_list: # not rules but has a value logger.debug( - "no rules found, returning feature default value", feature_name=feature_name, default_value=def_val + f"no rules found, returning feature default value, feature_name={feature_name}, default_value={default_value}" # noqa: E501 ) - return def_val + return default_value # look for first rule match - logger.debug("looking for rule match", feature_name=feature_name, feature_default_value=def_val) - return self._handle_rules(logger, feature_name, rules_context, def_val, rules_list) + logger.debug( + f"looking for rule match, feature_name={feature_name}, default_value={default_value}" + ) # noqa: E501 + return self._handle_rules( + feature_name=feature_name, rules_context=rules_context, default_value=default_value, rules=rules_list + ) diff --git a/aws_lambda_powertools/utilities/feature_toggles/exceptions.py b/aws_lambda_powertools/utilities/feature_toggles/exceptions.py new file mode 100644 index 0000000000..6fed03d805 --- /dev/null +++ b/aws_lambda_powertools/utilities/feature_toggles/exceptions.py @@ -0,0 +1,2 @@ +class ConfigurationException(Exception): + """When a a configuration store raises an exception on schema retrieval or parsing""" diff --git a/tests/functional/feature_toggles/test_feature_toggles.py b/tests/functional/feature_toggles/test_feature_toggles.py index 01ebae7bc6..73cb81e921 100644 --- a/tests/functional/feature_toggles/test_feature_toggles.py +++ b/tests/functional/feature_toggles/test_feature_toggles.py @@ -5,23 +5,6 @@ from aws_lambda_powertools.utilities.feature_toggles.configuration_store import ACTION, ConfigurationStore -class MockLogger: - def debug(self, message: str, **kwargs) -> None: - pass - - def info(self, message: str, **kwargs) -> None: - pass - - def warning(self, message: str, **kwargs) -> None: - pass - - def error(self, message: str, **kwargs) -> None: - pass - - def exception(self, message: str, **kwargs) -> None: - pass - - def init_configuration_store(mocker, mock_schema: Dict) -> ConfigurationStore: mocked_get_conf = mocker.patch("aws_lambda_powertools.utilities.parameters.AppConfigProvider.get") mocked_get_conf.return_value = mock_schema @@ -30,14 +13,13 @@ def init_configuration_store(mocker, mock_schema: Dict) -> ConfigurationStore: service="test_app", conf_name="test_conf_name", cache_seconds=600, - region_name="us-east-1", ) return conf_store # this test checks that we get correct value of feature that exists in the schema. # we also don't send an empty rules_context dict in this case -def test_toggles_no_restrictions(mocker): +def test_toggles_rule_does_not_match(mocker): expected_value = "True" mocked_app_config_schema = { "log_level": "DEBUG", @@ -61,11 +43,8 @@ def test_toggles_no_restrictions(mocker): }, } - logger = MockLogger() conf_store = init_configuration_store(mocker, mocked_app_config_schema) - toggle = conf_store.get_feature_toggle( - logger=logger, feature_name="my_feature", rules_context={}, default_value=False - ) + toggle = conf_store.get_feature_toggle(feature_name="my_feature", rules_context={}, default_value=False) assert str(toggle) == expected_value @@ -75,11 +54,8 @@ def test_toggles_no_restrictions_feature_does_not_exist(mocker): expected_value = False mocked_app_config_schema = {"log_level": "DEBUG", "features": {"my_fake_feature": {"default_value": "True"}}} - logger = MockLogger() conf_store = init_configuration_store(mocker, mocked_app_config_schema) - toggle = conf_store.get_feature_toggle( - logger=logger, feature_name="my_feature", rules_context={}, default_value=expected_value - ) + toggle = conf_store.get_feature_toggle(feature_name="my_feature", rules_context={}, default_value=expected_value) assert toggle == expected_value @@ -88,10 +64,9 @@ def test_toggles_no_restrictions_feature_does_not_exist(mocker): def test_toggles_no_rules(mocker): expected_value = "True" mocked_app_config_schema = {"log_level": "DEBUG", "features": {"my_feature": {"default_value": expected_value}}} - logger = MockLogger() conf_store = init_configuration_store(mocker, mocked_app_config_schema) toggle = conf_store.get_feature_toggle( - logger=logger, feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, default_value=False + feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, default_value=False ) assert str(toggle) == expected_value @@ -120,10 +95,8 @@ def test_toggles_restrictions_no_match(mocker): } }, } - logger = MockLogger() conf_store = init_configuration_store(mocker, mocked_app_config_schema) toggle = conf_store.get_feature_toggle( - logger=logger, feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, # rule will not match default_value=False, @@ -162,10 +135,8 @@ def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker): } }, } - logger = MockLogger() conf_store = init_configuration_store(mocker, mocked_app_config_schema) toggle = conf_store.get_feature_toggle( - logger=logger, feature_name="my_feature", rules_context={ "tenant_id": tenant_id_val, @@ -207,10 +178,9 @@ def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker): } }, } - logger = MockLogger() conf_store = init_configuration_store(mocker, mocked_app_config_schema) toggle = conf_store.get_feature_toggle( - logger=logger, feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, default_value=False + feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, default_value=False ) assert str(toggle) == expected_val @@ -269,11 +239,9 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl }, } - logger = MockLogger() conf_store = init_configuration_store(mocker, mocked_app_config_schema) # match first rule toggle = conf_store.get_feature_toggle( - logger=logger, feature_name="my_feature", rules_context={"tenant_id": "6", "username": "abcd"}, default_value=False, @@ -281,7 +249,6 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl assert str(toggle) == expected_value_first_check # match second rule toggle = conf_store.get_feature_toggle( - logger=logger, feature_name="my_feature", rules_context={"tenant_id": "4446", "username": "az"}, default_value=False, @@ -289,7 +256,6 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl assert str(toggle) == expected_value_second_check # match no rule toggle = conf_store.get_feature_toggle( - logger=logger, feature_name="my_feature", rules_context={"tenant_id": "11114446", "username": "ab"}, default_value=False, @@ -297,7 +263,6 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl assert str(toggle) == expected_value_third_check # feature doesn't exist toggle = conf_store.get_feature_toggle( - logger=logger, feature_name="my_fake_feature", rules_context={"tenant_id": "11114446", "username": "ab"}, default_value=expected_value_fourth_case, @@ -329,10 +294,8 @@ def test_toggles_match_rule_with_contains_action(mocker): } }, } - logger = MockLogger() conf_store = init_configuration_store(mocker, mocked_app_config_schema) toggle = conf_store.get_feature_toggle( - logger=logger, feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, # rule will match default_value=False, @@ -363,10 +326,8 @@ def test_toggles_no_match_rule_with_contains_action(mocker): } }, } - logger = MockLogger() conf_store = init_configuration_store(mocker, mocked_app_config_schema) toggle = conf_store.get_feature_toggle( - logger=logger, feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, # rule will not match default_value=False, From 10310a741239efcb1249a6397c3243f89516da4b Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Wed, 30 Jun 2021 21:04:30 +0300 Subject: [PATCH 03/12] fix init --- .../utilities/feature_toggles/configuration_store.py | 1 - 1 file changed, 1 deletion(-) diff --git a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py index 04700de1af..5e9f9b5291 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py +++ b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py @@ -46,7 +46,6 @@ def __init__( self._cache_seconds = cache_seconds self.logger = logger self._conf_name = conf_name - config = config or Config() self._conf_store = AppConfigProvider(environment=environment, application=service, config=config) def _validate_json_schema(self, schema: str) -> bool: From da9c27095e9fdbb379d5f86312c9dffb3188b748 Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Wed, 30 Jun 2021 21:15:58 +0300 Subject: [PATCH 04/12] fix config boto --- .../utilities/parameters/appconfig.py | 2 +- .../feature_toggles/test_feature_toggles.py | 45 +++++++++++-------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/aws_lambda_powertools/utilities/parameters/appconfig.py b/aws_lambda_powertools/utilities/parameters/appconfig.py index 4490e26036..63a8415f1e 100644 --- a/aws_lambda_powertools/utilities/parameters/appconfig.py +++ b/aws_lambda_powertools/utilities/parameters/appconfig.py @@ -149,7 +149,7 @@ def get_app_config( >>> print(value) My configuration value - **Retrieves a confiugration value and decodes it using a JSON decoder** + **Retrieves a configuration value and decodes it using a JSON decoder** >>> from aws_lambda_powertools.utilities.parameters import get_parameter >>> diff --git a/tests/functional/feature_toggles/test_feature_toggles.py b/tests/functional/feature_toggles/test_feature_toggles.py index 73cb81e921..b61e4e5285 100644 --- a/tests/functional/feature_toggles/test_feature_toggles.py +++ b/tests/functional/feature_toggles/test_feature_toggles.py @@ -1,11 +1,17 @@ from typing import Dict import pytest # noqa: F401 +from botocore.config import Config from aws_lambda_powertools.utilities.feature_toggles.configuration_store import ACTION, ConfigurationStore -def init_configuration_store(mocker, mock_schema: Dict) -> ConfigurationStore: +@pytest.fixture(scope="module") +def config(): + return Config(region_name="us-east-1") + + +def init_configuration_store(mocker, mock_schema: Dict, config: Config) -> ConfigurationStore: mocked_get_conf = mocker.patch("aws_lambda_powertools.utilities.parameters.AppConfigProvider.get") mocked_get_conf.return_value = mock_schema conf_store: ConfigurationStore = ConfigurationStore( @@ -13,13 +19,14 @@ def init_configuration_store(mocker, mock_schema: Dict) -> ConfigurationStore: service="test_app", conf_name="test_conf_name", cache_seconds=600, + config=config, ) return conf_store # this test checks that we get correct value of feature that exists in the schema. # we also don't send an empty rules_context dict in this case -def test_toggles_rule_does_not_match(mocker): +def test_toggles_rule_does_not_match(mocker, config): expected_value = "True" mocked_app_config_schema = { "log_level": "DEBUG", @@ -43,28 +50,28 @@ def test_toggles_rule_does_not_match(mocker): }, } - conf_store = init_configuration_store(mocker, mocked_app_config_schema) + conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle(feature_name="my_feature", rules_context={}, default_value=False) assert str(toggle) == expected_value # this test checks that if you try to get a feature that doesn't exist in the schema, # you get the default value of False that was sent to the get_feature_toggle API -def test_toggles_no_restrictions_feature_does_not_exist(mocker): +def test_toggles_no_restrictions_feature_does_not_exist(mocker, config): expected_value = False mocked_app_config_schema = {"log_level": "DEBUG", "features": {"my_fake_feature": {"default_value": "True"}}} - conf_store = init_configuration_store(mocker, mocked_app_config_schema) + conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle(feature_name="my_feature", rules_context={}, default_value=expected_value) assert toggle == expected_value # check that feature match works when they are no rules and we send rules_context. # default value is False but the feature has a True default_value. -def test_toggles_no_rules(mocker): +def test_toggles_no_rules(mocker, config): expected_value = "True" mocked_app_config_schema = {"log_level": "DEBUG", "features": {"my_feature": {"default_value": expected_value}}} - conf_store = init_configuration_store(mocker, mocked_app_config_schema) + conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, default_value=False ) @@ -72,7 +79,7 @@ def test_toggles_no_rules(mocker): # check a case where the feature exists but the rule doesn't match so we revert to the default value of the feature -def test_toggles_restrictions_no_match(mocker): +def test_toggles_restrictions_no_match(mocker, config): expected_value = "True" mocked_app_config_schema = { "log_level": "DEBUG", @@ -95,7 +102,7 @@ def test_toggles_restrictions_no_match(mocker): } }, } - conf_store = init_configuration_store(mocker, mocked_app_config_schema) + conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, # rule will not match @@ -105,7 +112,7 @@ def test_toggles_restrictions_no_match(mocker): # check that a rule can match when it has multiple restrictions, see rule name for further explanation -def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker): +def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker, config): expected_value = "False" tenant_id_val = "6" username_val = "a" @@ -135,7 +142,7 @@ def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker): } }, } - conf_store = init_configuration_store(mocker, mocked_app_config_schema) + conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={ @@ -150,7 +157,7 @@ def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker): # check a case when rule doesn't match and it has multiple restrictions, # different tenant id causes the rule to not match. # default value of the feature in this case is True -def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker): +def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker, config): expected_val = "True" mocked_app_config_schema = { "log_level": "DEBUG", @@ -178,7 +185,7 @@ def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker): } }, } - conf_store = init_configuration_store(mocker, mocked_app_config_schema) + conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, default_value=False ) @@ -186,7 +193,7 @@ def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker): # check rule match for multiple of action types -def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multiple_restrictions(mocker): +def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multiple_restrictions(mocker, config): expected_value_first_check = "True" expected_value_second_check = "False" expected_value_third_check = "False" @@ -239,7 +246,7 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl }, } - conf_store = init_configuration_store(mocker, mocked_app_config_schema) + conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) # match first rule toggle = conf_store.get_feature_toggle( feature_name="my_feature", @@ -271,7 +278,7 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl # check a case where the feature exists but the rule doesn't match so we revert to the default value of the feature -def test_toggles_match_rule_with_contains_action(mocker): +def test_toggles_match_rule_with_contains_action(mocker, config): expected_value = True mocked_app_config_schema = { "log_level": "DEBUG", @@ -294,7 +301,7 @@ def test_toggles_match_rule_with_contains_action(mocker): } }, } - conf_store = init_configuration_store(mocker, mocked_app_config_schema) + conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, # rule will match @@ -303,7 +310,7 @@ def test_toggles_match_rule_with_contains_action(mocker): assert toggle == expected_value -def test_toggles_no_match_rule_with_contains_action(mocker): +def test_toggles_no_match_rule_with_contains_action(mocker, config): expected_value = False mocked_app_config_schema = { "log_level": "DEBUG", @@ -326,7 +333,7 @@ def test_toggles_no_match_rule_with_contains_action(mocker): } }, } - conf_store = init_configuration_store(mocker, mocked_app_config_schema) + conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, # rule will not match From a21081dc29b21a8fc359d8b970f5f563354aca1f Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Fri, 2 Jul 2021 20:09:06 +0300 Subject: [PATCH 05/12] rename default value --- .../feature_toggles/configuration_store.py | 38 ++++++------ .../feature_toggles/test_feature_toggles.py | 58 +++++++++---------- 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py index 5e9f9b5291..10c2bfef85 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py +++ b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py @@ -12,10 +12,10 @@ TRANSFORM_TYPE = "json" FEATURES_KEY = "features" RULES_KEY = "rules" -DEFAULT_VAL_KEY = "default_value" +DEFAULT_VAL_KEY = "feature_default_value" RESTRICTIONS_KEY = "restrictions" RULE_NAME_KEY = "name" -RULE_DEFAULT_VALUE = "default_value" +RULE_DEFAULT_VALUE = "rule_default_value" RESTRICTION_KEY = "key" RESTRICTION_VALUE = "value" RESTRICTION_ACTION = "action" @@ -73,7 +73,7 @@ def _handle_rules( *, feature_name: str, rules_context: Dict[str, Any], - default_value: bool, + feature_default_value: bool, rules: List[Dict[str, Any]], ) -> bool: for rule in rules: @@ -88,21 +88,21 @@ def _handle_rules( restriction.get(RESTRICTION_ACTION), restriction.get(RESTRICTION_VALUE), context_value ): logger.debug( - f"rule did not match action, rule_name={rule_name}, default_value={rule_default_value}, feature_name={feature_name}, context_value={str(context_value)}" # noqa: E501 + f"rule did not match action, rule_name={rule_name}, rule_default_value={rule_default_value}, feature_name={feature_name}, context_value={str(context_value)}" # noqa: E501 ) is_match = False # rules doesn't match restriction break # if we got here, all restrictions match if is_match: logger.debug( - f"rule matched, rule_name={rule_name}, default_value={rule_default_value}, feature_name={feature_name}" # noqa: E501 + f"rule matched, rule_name={rule_name}, rule_default_value={rule_default_value}, feature_name={feature_name}" # noqa: E501 ) return rule_default_value # no rule matched, return default value of feature logger.debug( - f"no rule matched, returning default value of feature, default_value={default_value}, feature_name={feature_name}" # noqa: E501 + f"no rule matched, returning default value of feature, feature_default_value={feature_default_value}, feature_name={feature_name}" # noqa: E501 ) - return default_value + return feature_default_value def get_configuration(self) -> Dict[str, Any]: """Get configuration string from AWs AppConfig and returned the parsed JSON dictionary @@ -130,7 +130,7 @@ def get_configuration(self) -> Dict[str, Any]: raise ConfigurationException(error_str) return schema - def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any], default_value: bool) -> bool: + def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any], value_if_missing: bool) -> bool: """get a feature toggle boolean value. Value is calculated according to a set of rules and conditions. see below for explanation. @@ -138,16 +138,16 @@ def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any] feature_name (str): feature name that you wish to fetch rules_context (Dict[str, Any]): dict of attributes that you would like to match the rules against, can be {'tenant_id: 'X', 'username':' 'Y', 'region': 'Z'} etc. - default_value (bool): this will be the returned value in case the feature toggle doesn't exist in the schema + value_if_missing (bool): this will be the returned value in case the feature toggle doesn't exist in the schema or there has been an error while fetching the configuration from appconfig Returns: bool: calculated feature toggle value. several possibilities: 1. if the feature doesn't appear in the schema or there has been an error fetching the - configuration -> error log would appear and default_value is returned - 2. feature exists and has no rules or no rules have matched -> return default_value of + configuration -> warning log would appear and value_if_missing is returned + 2. feature exists and has no rules or no rules have matched -> return feature_default_value of the defined feature - 3. feature exists and a rule matches -> default_value of rule is returned + 3. feature exists and a rule matches -> rule_default_value of rule is returned """ try: toggles_dict: Dict[str, Any] = self.get_configuration() @@ -158,22 +158,22 @@ def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any] feature: Dict[str, Dict] = toggles_dict.get(FEATURES_KEY, {}).get(feature_name, None) if feature is None: logger.warning( - f"feature does not appear in configuration, using provided default value, feature_name={feature_name}, default_value={default_value}" # noqa: E501 + f"feature does not appear in configuration, using provided default value, feature_name={feature_name}, value_if_missing={value_if_missing}" # noqa: E501 ) - return default_value + return value_if_missing rules_list = feature.get(RULES_KEY, []) - default_value = feature.get(DEFAULT_VAL_KEY) + feature_default_value = feature.get(DEFAULT_VAL_KEY) if not rules_list: # not rules but has a value logger.debug( - f"no rules found, returning feature default value, feature_name={feature_name}, default_value={default_value}" # noqa: E501 + f"no rules found, returning feature default value, feature_name={feature_name}, default_value={feature_default_value}" # noqa: E501 ) - return default_value + return feature_default_value # look for first rule match logger.debug( - f"looking for rule match, feature_name={feature_name}, default_value={default_value}" + f"looking for rule match, feature_name={feature_name}, feature_default_value={feature_default_value}" ) # noqa: E501 return self._handle_rules( - feature_name=feature_name, rules_context=rules_context, default_value=default_value, rules=rules_list + feature_name=feature_name, rules_context=rules_context, feature_default_value=feature_default_value, rules=rules_list ) diff --git a/tests/functional/feature_toggles/test_feature_toggles.py b/tests/functional/feature_toggles/test_feature_toggles.py index b61e4e5285..90da38b5f5 100644 --- a/tests/functional/feature_toggles/test_feature_toggles.py +++ b/tests/functional/feature_toggles/test_feature_toggles.py @@ -32,11 +32,11 @@ def test_toggles_rule_does_not_match(mocker, config): "log_level": "DEBUG", "features": { "my_feature": { - "default_value": expected_value, + "feature_default_value": expected_value, "rules": [ { "name": "tenant id equals 345345435", - "default_value": "False", + "rule_default_value": "False", "restrictions": [ { "action": ACTION.EQUALS.value, @@ -51,7 +51,7 @@ def test_toggles_rule_does_not_match(mocker, config): } conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) - toggle = conf_store.get_feature_toggle(feature_name="my_feature", rules_context={}, default_value=False) + toggle = conf_store.get_feature_toggle(feature_name="my_feature", rules_context={}, value_if_missing=False) assert str(toggle) == expected_value @@ -59,10 +59,10 @@ def test_toggles_rule_does_not_match(mocker, config): # you get the default value of False that was sent to the get_feature_toggle API def test_toggles_no_restrictions_feature_does_not_exist(mocker, config): expected_value = False - mocked_app_config_schema = {"log_level": "DEBUG", "features": {"my_fake_feature": {"default_value": "True"}}} + mocked_app_config_schema = {"log_level": "DEBUG", "features": {"my_fake_feature": {"feature_default_value": "True"}}} conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) - toggle = conf_store.get_feature_toggle(feature_name="my_feature", rules_context={}, default_value=expected_value) + toggle = conf_store.get_feature_toggle(feature_name="my_feature", rules_context={}, value_if_missing=expected_value) assert toggle == expected_value @@ -70,10 +70,10 @@ def test_toggles_no_restrictions_feature_does_not_exist(mocker, config): # default value is False but the feature has a True default_value. def test_toggles_no_rules(mocker, config): expected_value = "True" - mocked_app_config_schema = {"log_level": "DEBUG", "features": {"my_feature": {"default_value": expected_value}}} + mocked_app_config_schema = {"log_level": "DEBUG", "features": {"my_feature": {"feature_default_value": expected_value}}} conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle( - feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, default_value=False + feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, value_if_missing=False ) assert str(toggle) == expected_value @@ -85,11 +85,11 @@ def test_toggles_restrictions_no_match(mocker, config): "log_level": "DEBUG", "features": { "my_feature": { - "default_value": expected_value, + "feature_default_value": expected_value, "rules": [ { "name": "tenant id equals 345345435", - "default_value": "False", + "rule_default_value": "False", "restrictions": [ { "action": ACTION.EQUALS.value, @@ -106,7 +106,7 @@ def test_toggles_restrictions_no_match(mocker, config): toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, # rule will not match - default_value=False, + value_if_missing=False, ) assert str(toggle) == expected_value @@ -120,11 +120,11 @@ def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker, con "log_level": "DEBUG", "features": { "my_feature": { - "default_value": "True", + "feature_default_value": "True", "rules": [ { "name": "tenant id equals 6 and username is a", - "default_value": expected_value, + "rule_default_value": expected_value, "restrictions": [ { "action": ACTION.EQUALS.value, # this rule will match, it has multiple restrictions @@ -149,7 +149,7 @@ def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker, con "tenant_id": tenant_id_val, "username": username_val, }, - default_value=True, + value_if_missing=True, ) assert str(toggle) == expected_value @@ -163,11 +163,11 @@ def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker, "log_level": "DEBUG", "features": { "my_feature": { - "default_value": expected_val, + "feature_default_value": expected_val, "rules": [ { "name": "tenant id equals 645654 and username is a", # rule will not match - "default_value": "False", + "rule_default_value": "False", "restrictions": [ { "action": ACTION.EQUALS.value, @@ -187,7 +187,7 @@ def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker, } conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle( - feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, default_value=False + feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, value_if_missing=False ) assert str(toggle) == expected_val @@ -202,11 +202,11 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl "log_level": "DEBUG", "features": { "my_feature": { - "default_value": expected_value_third_check, + "feature_default_value": expected_value_third_check, "rules": [ { "name": "tenant id equals 6 and username startswith a", - "default_value": expected_value_first_check, + "rule_default_value": expected_value_first_check, "restrictions": [ { "action": ACTION.EQUALS.value, @@ -222,7 +222,7 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl }, { "name": "tenant id equals 4446 and username startswith a and endswith z", - "default_value": expected_value_second_check, + "rule_default_value": expected_value_second_check, "restrictions": [ { "action": ACTION.EQUALS.value, @@ -251,28 +251,28 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "6", "username": "abcd"}, - default_value=False, + value_if_missing=False, ) assert str(toggle) == expected_value_first_check # match second rule toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "4446", "username": "az"}, - default_value=False, + value_if_missing=False, ) assert str(toggle) == expected_value_second_check # match no rule toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "11114446", "username": "ab"}, - default_value=False, + value_if_missing=False, ) assert str(toggle) == expected_value_third_check # feature doesn't exist toggle = conf_store.get_feature_toggle( feature_name="my_fake_feature", rules_context={"tenant_id": "11114446", "username": "ab"}, - default_value=expected_value_fourth_case, + value_if_missing=expected_value_fourth_case, ) assert toggle == expected_value_fourth_case @@ -284,11 +284,11 @@ def test_toggles_match_rule_with_contains_action(mocker, config): "log_level": "DEBUG", "features": { "my_feature": { - "default_value": expected_value, + "feature_default_value": expected_value, "rules": [ { "name": "tenant id equals 345345435", - "default_value": True, + "rule_default_value": True, "restrictions": [ { "action": ACTION.CONTAINS.value, @@ -305,7 +305,7 @@ def test_toggles_match_rule_with_contains_action(mocker, config): toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, # rule will match - default_value=False, + value_if_missing=False, ) assert toggle == expected_value @@ -316,11 +316,11 @@ def test_toggles_no_match_rule_with_contains_action(mocker, config): "log_level": "DEBUG", "features": { "my_feature": { - "default_value": expected_value, + "feature_default_value": expected_value, "rules": [ { "name": "tenant id equals 345345435", - "default_value": True, + "rule_default_value": True, "restrictions": [ { "action": ACTION.CONTAINS.value, @@ -337,6 +337,6 @@ def test_toggles_no_match_rule_with_contains_action(mocker, config): toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, # rule will not match - default_value=False, + value_if_missing=False, ) assert toggle == expected_value From b9571f3afa23014ee58158e45c85aa85bcee9bf6 Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Fri, 2 Jul 2021 20:14:26 +0300 Subject: [PATCH 06/12] formatting fix --- .../feature_toggles/configuration_store.py | 22 +++++++++++-------- .../feature_toggles/test_feature_toggles.py | 11 ++-------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py index 10c2bfef85..080677fe49 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py +++ b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py @@ -48,8 +48,8 @@ def __init__( self._conf_name = conf_name self._conf_store = AppConfigProvider(environment=environment, application=service, config=config) - def _validate_json_schema(self, schema: str) -> bool: - ## todo + def _validate_json_schema(self, schema: Dict[str, Any]) -> bool: + # return True def _match_by_action(self, action: str, restriction_value: Any, context_value: Any) -> bool: @@ -138,13 +138,14 @@ def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any] feature_name (str): feature name that you wish to fetch rules_context (Dict[str, Any]): dict of attributes that you would like to match the rules against, can be {'tenant_id: 'X', 'username':' 'Y', 'region': 'Z'} etc. - value_if_missing (bool): this will be the returned value in case the feature toggle doesn't exist in the schema - or there has been an error while fetching the configuration from appconfig + value_if_missing (bool): this will be the returned value in case the feature toggle doesn't exist in + the schema or there has been an error while fetching the + configuration from appconfig Returns: bool: calculated feature toggle value. several possibilities: 1. if the feature doesn't appear in the schema or there has been an error fetching the - configuration -> warning log would appear and value_if_missing is returned + configuration -> error/warning log would appear and value_if_missing is returned 2. feature exists and has no rules or no rules have matched -> return feature_default_value of the defined feature 3. feature exists and a rule matches -> rule_default_value of rule is returned @@ -152,13 +153,13 @@ def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any] try: toggles_dict: Dict[str, Any] = self.get_configuration() except ConfigurationException: - logger.warning("unable to get feature toggles JSON, returning provided default value") - return default_value + logger.error("unable to get feature toggles JSON, returning provided value_if_missing value") # noqa: E501 + return value_if_missing feature: Dict[str, Dict] = toggles_dict.get(FEATURES_KEY, {}).get(feature_name, None) if feature is None: logger.warning( - f"feature does not appear in configuration, using provided default value, feature_name={feature_name}, value_if_missing={value_if_missing}" # noqa: E501 + f"feature does not appear in configuration, using provided value_if_missing, feature_name={feature_name}, value_if_missing={value_if_missing}" # noqa: E501 ) return value_if_missing @@ -175,5 +176,8 @@ def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any] f"looking for rule match, feature_name={feature_name}, feature_default_value={feature_default_value}" ) # noqa: E501 return self._handle_rules( - feature_name=feature_name, rules_context=rules_context, feature_default_value=feature_default_value, rules=rules_list + feature_name=feature_name, + rules_context=rules_context, + feature_default_value=feature_default_value, + rules=rules_list, ) diff --git a/tests/functional/feature_toggles/test_feature_toggles.py b/tests/functional/feature_toggles/test_feature_toggles.py index 90da38b5f5..3447168c1a 100644 --- a/tests/functional/feature_toggles/test_feature_toggles.py +++ b/tests/functional/feature_toggles/test_feature_toggles.py @@ -29,7 +29,6 @@ def init_configuration_store(mocker, mock_schema: Dict, config: Config) -> Confi def test_toggles_rule_does_not_match(mocker, config): expected_value = "True" mocked_app_config_schema = { - "log_level": "DEBUG", "features": { "my_feature": { "feature_default_value": expected_value, @@ -59,7 +58,7 @@ def test_toggles_rule_does_not_match(mocker, config): # you get the default value of False that was sent to the get_feature_toggle API def test_toggles_no_restrictions_feature_does_not_exist(mocker, config): expected_value = False - mocked_app_config_schema = {"log_level": "DEBUG", "features": {"my_fake_feature": {"feature_default_value": "True"}}} + mocked_app_config_schema = {"features": {"my_fake_feature": {"feature_default_value": "True"}}} conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle(feature_name="my_feature", rules_context={}, value_if_missing=expected_value) @@ -70,7 +69,7 @@ def test_toggles_no_restrictions_feature_does_not_exist(mocker, config): # default value is False but the feature has a True default_value. def test_toggles_no_rules(mocker, config): expected_value = "True" - mocked_app_config_schema = {"log_level": "DEBUG", "features": {"my_feature": {"feature_default_value": expected_value}}} + mocked_app_config_schema = {"features": {"my_feature": {"feature_default_value": expected_value}}} conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, value_if_missing=False @@ -82,7 +81,6 @@ def test_toggles_no_rules(mocker, config): def test_toggles_restrictions_no_match(mocker, config): expected_value = "True" mocked_app_config_schema = { - "log_level": "DEBUG", "features": { "my_feature": { "feature_default_value": expected_value, @@ -117,7 +115,6 @@ def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker, con tenant_id_val = "6" username_val = "a" mocked_app_config_schema = { - "log_level": "DEBUG", "features": { "my_feature": { "feature_default_value": "True", @@ -160,7 +157,6 @@ def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker, con def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker, config): expected_val = "True" mocked_app_config_schema = { - "log_level": "DEBUG", "features": { "my_feature": { "feature_default_value": expected_val, @@ -199,7 +195,6 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl expected_value_third_check = "False" expected_value_fourth_case = False mocked_app_config_schema = { - "log_level": "DEBUG", "features": { "my_feature": { "feature_default_value": expected_value_third_check, @@ -281,7 +276,6 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl def test_toggles_match_rule_with_contains_action(mocker, config): expected_value = True mocked_app_config_schema = { - "log_level": "DEBUG", "features": { "my_feature": { "feature_default_value": expected_value, @@ -313,7 +307,6 @@ def test_toggles_match_rule_with_contains_action(mocker, config): def test_toggles_no_match_rule_with_contains_action(mocker, config): expected_value = False mocked_app_config_schema = { - "log_level": "DEBUG", "features": { "my_feature": { "feature_default_value": expected_value, From 78f6eb3055f4b8cd1a07c892cbf1c42432d93e71 Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Fri, 2 Jul 2021 22:57:40 +0300 Subject: [PATCH 07/12] cr fixes,. added token validation --- .../utilities/feature_toggles/__init__.py | 4 +- .../feature_toggles/configuration_store.py | 63 ++--- .../utilities/feature_toggles/schema.py | 83 ++++++ .../feature_toggles/test_feature_toggles.py | 71 ++--- .../feature_toggles/test_schema_validation.py | 264 ++++++++++++++++++ 5 files changed, 408 insertions(+), 77 deletions(-) create mode 100644 aws_lambda_powertools/utilities/feature_toggles/schema.py create mode 100644 tests/functional/feature_toggles/test_schema_validation.py diff --git a/aws_lambda_powertools/utilities/feature_toggles/__init__.py b/aws_lambda_powertools/utilities/feature_toggles/__init__.py index 36789f6362..be5b024539 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/__init__.py +++ b/aws_lambda_powertools/utilities/feature_toggles/__init__.py @@ -1,10 +1,12 @@ """Advanced feature toggles utility """ -from .configuration_store import ACTION, ConfigurationStore +from .configuration_store import ConfigurationStore from .exceptions import ConfigurationException +from .schema import ACTION, SchemaValidator __all__ = [ "ConfigurationException", "ConfigurationStore", "ACTION", + "SchemaValidator", ] diff --git a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py index 080677fe49..1413492008 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py +++ b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py @@ -1,32 +1,15 @@ # pylint: disable=no-name-in-module,line-too-long import logging -from enum import Enum from typing import Any, Dict, List, Optional from botocore.config import Config from aws_lambda_powertools.utilities.parameters import AppConfigProvider, GetParameterError, TransformParameterError +from . import schema from .exceptions import ConfigurationException TRANSFORM_TYPE = "json" -FEATURES_KEY = "features" -RULES_KEY = "rules" -DEFAULT_VAL_KEY = "feature_default_value" -RESTRICTIONS_KEY = "restrictions" -RULE_NAME_KEY = "name" -RULE_DEFAULT_VALUE = "rule_default_value" -RESTRICTION_KEY = "key" -RESTRICTION_VALUE = "value" -RESTRICTION_ACTION = "action" - - -class ACTION(str, Enum): - EQUALS = "EQUALS" - STARTSWITH = "STARTSWITH" - ENDSWITH = "ENDSWITH" - CONTAINS = "CONTAINS" - logger = logging.getLogger(__name__) @@ -44,28 +27,26 @@ def __init__( cache_seconds (int): cache expiration time, how often to call AppConfig to fetch latest configuration """ self._cache_seconds = cache_seconds - self.logger = logger + self._logger = logger self._conf_name = conf_name + self._schema_validator = schema.SchemaValidator(self._logger) self._conf_store = AppConfigProvider(environment=environment, application=service, config=config) - def _validate_json_schema(self, schema: Dict[str, Any]) -> bool: - # - return True - def _match_by_action(self, action: str, restriction_value: Any, context_value: Any) -> bool: if not context_value: return False mapping_by_action = { - ACTION.EQUALS.value: lambda a, b: a == b, - ACTION.STARTSWITH.value: lambda a, b: a.startswith(b), - ACTION.ENDSWITH.value: lambda a, b: a.endswith(b), - ACTION.CONTAINS.value: lambda a, b: a in b, + schema.ACTION.EQUALS.value: lambda a, b: a == b, + schema.ACTION.STARTSWITH.value: lambda a, b: a.startswith(b), + schema.ACTION.ENDSWITH.value: lambda a, b: a.endswith(b), + schema.ACTION.CONTAINS.value: lambda a, b: a in b, } try: func = mapping_by_action.get(action, lambda a, b: False) return func(context_value, restriction_value) - except Exception: + except Exception as exc: + self._logger.error(f"caught exception while matching action, action={action}, exception={str(exc)}") return False def _handle_rules( @@ -77,15 +58,17 @@ def _handle_rules( rules: List[Dict[str, Any]], ) -> bool: for rule in rules: - rule_name = rule.get(RULE_NAME_KEY, "") - rule_default_value = rule.get(RULE_DEFAULT_VALUE) + rule_name = rule.get(schema.RULE_NAME_KEY, "") + rule_default_value = rule.get(schema.RULE_DEFAULT_VALUE) is_match = True - restrictions: Dict[str, str] = rule.get(RESTRICTIONS_KEY) + restrictions: Dict[str, str] = rule.get(schema.RESTRICTIONS_KEY) for restriction in restrictions: - context_value = rules_context.get(restriction.get(RESTRICTION_KEY, ""), "") + context_value = rules_context.get(restriction.get(schema.RESTRICTION_KEY)) if not self._match_by_action( - restriction.get(RESTRICTION_ACTION), restriction.get(RESTRICTION_VALUE), context_value + restriction.get(schema.RESTRICTION_ACTION), + restriction.get(schema.RESTRICTION_VALUE), + context_value, ): logger.debug( f"rule did not match action, rule_name={rule_name}, rule_default_value={rule_default_value}, feature_name={feature_name}, context_value={str(context_value)}" # noqa: E501 @@ -121,13 +104,11 @@ def get_configuration(self) -> Dict[str, Any]: ) # parse result conf as JSON, keep in cache for self.max_age seconds except (GetParameterError, TransformParameterError) as exc: error_str = f"unable to get AWS AppConfig configuration file, exception={str(exc)}" - logger.error(error_str) + self._logger.error(error_str) raise ConfigurationException(error_str) + # validate schema - if not self._validate_json_schema(schema): - error_str = "AWS AppConfig schema failed validation" - logger.error(error_str) - raise ConfigurationException(error_str) + self._schema_validator.validate_json_schema(schema) return schema def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any], value_if_missing: bool) -> bool: @@ -156,15 +137,15 @@ def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any] logger.error("unable to get feature toggles JSON, returning provided value_if_missing value") # noqa: E501 return value_if_missing - feature: Dict[str, Dict] = toggles_dict.get(FEATURES_KEY, {}).get(feature_name, None) + feature: Dict[str, Dict] = toggles_dict.get(schema.FEATURES_KEY, {}).get(feature_name, None) if feature is None: logger.warning( f"feature does not appear in configuration, using provided value_if_missing, feature_name={feature_name}, value_if_missing={value_if_missing}" # noqa: E501 ) return value_if_missing - rules_list = feature.get(RULES_KEY, []) - feature_default_value = feature.get(DEFAULT_VAL_KEY) + rules_list = feature.get(schema.RULES_KEY) + feature_default_value = feature.get(schema.FEATURE_DEFAULT_VAL_KEY) if not rules_list: # not rules but has a value logger.debug( diff --git a/aws_lambda_powertools/utilities/feature_toggles/schema.py b/aws_lambda_powertools/utilities/feature_toggles/schema.py new file mode 100644 index 0000000000..35500da176 --- /dev/null +++ b/aws_lambda_powertools/utilities/feature_toggles/schema.py @@ -0,0 +1,83 @@ +from enum import Enum +from typing import Any, Dict + +from .exceptions import ConfigurationException + +FEATURES_KEY = "features" +RULES_KEY = "rules" +FEATURE_DEFAULT_VAL_KEY = "feature_default_value" +RESTRICTIONS_KEY = "restrictions" +RULE_NAME_KEY = "rule_name" +RULE_DEFAULT_VALUE = "value_when_applies" +RESTRICTION_KEY = "key" +RESTRICTION_VALUE = "value" +RESTRICTION_ACTION = "action" + + +class ACTION(str, Enum): + EQUALS = "EQUALS" + STARTSWITH = "STARTSWITH" + ENDSWITH = "ENDSWITH" + CONTAINS = "CONTAINS" + + +class SchemaValidator: + def __init__(self, logger: object): + self._logger = logger + + def _raise_conf_exc(self, error_str: str) -> None: + self._logger.error(error_str) + raise ConfigurationException(error_str) + + def _validate_restriction(self, rule_name: str, restriction: Dict[str, str]) -> None: + if not restriction or not isinstance(restriction, dict): + self._raise_conf_exc(f"invalid restriction type, not a dictionary, rule_name={rule_name}") + action = restriction.get(RESTRICTION_ACTION, "") + if action not in [ACTION.EQUALS.value, ACTION.STARTSWITH.value, ACTION.ENDSWITH.value, ACTION.CONTAINS.value]: + self._raise_conf_exc(f"invalid action value, rule_name={rule_name}, action={action}") + key = restriction.get(RESTRICTION_KEY, "") + if not key or not isinstance(key, str): + self._raise_conf_exc(f"invalid key value, key has to be a non empty string, rule_name={rule_name}") + value = restriction.get(RESTRICTION_VALUE, "") + if not value: + self._raise_conf_exc(f"missing restriction value, rule_name={rule_name}") + + def _validate_rule(self, feature_name: str, rule: Dict[str, Any]) -> None: + if not rule or not isinstance(rule, dict): + self._raise_conf_exc(f"feature rule is not a dictionary, feature_name={feature_name}") + rule_name = rule.get(RULE_NAME_KEY) + if not rule_name or rule_name is None or not isinstance(rule_name, str): + self._raise_conf_exc(f"invalid rule_name, feature_name={feature_name}") + rule_default_value = rule.get(RULE_DEFAULT_VALUE) + if rule_default_value is None or not isinstance(rule_default_value, bool): + self._raise_conf_exc(f"invalid rule_default_value, rule_name={rule_name}") + restrictions = rule.get(RESTRICTIONS_KEY, {}) + if not restrictions or not isinstance(restrictions, list): + self._raise_conf_exc(f"invalid restrictions, rule_name={rule_name}") + # validate restrictions + for restriction in restrictions: + self._validate_restriction(rule_name, restriction) + + def _validate_feature(self, feature_name: str, feature_dict_def: Dict[str, Any]) -> None: + if not feature_dict_def or not isinstance(feature_dict_def, dict): + self._raise_conf_exc(f"invalid AWS AppConfig JSON schema detected, feature {feature_name} is invalid") + feature_default_value = feature_dict_def.get(FEATURE_DEFAULT_VAL_KEY) + if feature_default_value is None or not isinstance(feature_default_value, bool): + self._raise_conf_exc(f"missing feature_default_value for feature, feature_name={feature_name}") + # validate rules + rules = feature_dict_def.get(RULES_KEY, []) + if not rules: + return + if not isinstance(rules, list): + self._raise_conf_exc(f"feature rules is not a list, feature_name={feature_name}") + for rule in rules: + self._validate_rule(feature_name, rule) + + def validate_json_schema(self, schema: Dict[str, Any]) -> None: + if not isinstance(schema, dict): + self._raise_conf_exc("invalid AWS AppConfig JSON schema detected, root schema is not a dictionary") + features_dict: Dict = schema.get(FEATURES_KEY) + if not isinstance(features_dict, dict): + self._raise_conf_exc("invalid AWS AppConfig JSON schema detected, missing features dictionary") + for feature_name, feature_dict_def in features_dict.items(): + self._validate_feature(feature_name, feature_dict_def) diff --git a/tests/functional/feature_toggles/test_feature_toggles.py b/tests/functional/feature_toggles/test_feature_toggles.py index 3447168c1a..8289451b0c 100644 --- a/tests/functional/feature_toggles/test_feature_toggles.py +++ b/tests/functional/feature_toggles/test_feature_toggles.py @@ -3,7 +3,8 @@ import pytest # noqa: F401 from botocore.config import Config -from aws_lambda_powertools.utilities.feature_toggles.configuration_store import ACTION, ConfigurationStore +from aws_lambda_powertools.utilities.feature_toggles.configuration_store import ConfigurationStore +from aws_lambda_powertools.utilities.feature_toggles.schema import ACTION @pytest.fixture(scope="module") @@ -27,15 +28,15 @@ def init_configuration_store(mocker, mock_schema: Dict, config: Config) -> Confi # this test checks that we get correct value of feature that exists in the schema. # we also don't send an empty rules_context dict in this case def test_toggles_rule_does_not_match(mocker, config): - expected_value = "True" + expected_value = True mocked_app_config_schema = { "features": { "my_feature": { "feature_default_value": expected_value, "rules": [ { - "name": "tenant id equals 345345435", - "rule_default_value": "False", + "rule_name": "tenant id equals 345345435", + "value_when_applies": False, "restrictions": [ { "action": ACTION.EQUALS.value, @@ -51,14 +52,14 @@ def test_toggles_rule_does_not_match(mocker, config): conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle(feature_name="my_feature", rules_context={}, value_if_missing=False) - assert str(toggle) == expected_value + assert toggle == expected_value # this test checks that if you try to get a feature that doesn't exist in the schema, # you get the default value of False that was sent to the get_feature_toggle API def test_toggles_no_restrictions_feature_does_not_exist(mocker, config): expected_value = False - mocked_app_config_schema = {"features": {"my_fake_feature": {"feature_default_value": "True"}}} + mocked_app_config_schema = {"features": {"my_fake_feature": {"feature_default_value": True}}} conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle(feature_name="my_feature", rules_context={}, value_if_missing=expected_value) @@ -68,26 +69,26 @@ def test_toggles_no_restrictions_feature_does_not_exist(mocker, config): # check that feature match works when they are no rules and we send rules_context. # default value is False but the feature has a True default_value. def test_toggles_no_rules(mocker, config): - expected_value = "True" + expected_value = True mocked_app_config_schema = {"features": {"my_feature": {"feature_default_value": expected_value}}} conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, value_if_missing=False ) - assert str(toggle) == expected_value + assert toggle == expected_value # check a case where the feature exists but the rule doesn't match so we revert to the default value of the feature def test_toggles_restrictions_no_match(mocker, config): - expected_value = "True" + expected_value = True mocked_app_config_schema = { "features": { "my_feature": { "feature_default_value": expected_value, "rules": [ { - "name": "tenant id equals 345345435", - "rule_default_value": "False", + "rule_name": "tenant id equals 345345435", + "value_when_applies": False, "restrictions": [ { "action": ACTION.EQUALS.value, @@ -106,22 +107,22 @@ def test_toggles_restrictions_no_match(mocker, config): rules_context={"tenant_id": "6", "username": "a"}, # rule will not match value_if_missing=False, ) - assert str(toggle) == expected_value + assert toggle == expected_value # check that a rule can match when it has multiple restrictions, see rule name for further explanation def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker, config): - expected_value = "False" + expected_value = False tenant_id_val = "6" username_val = "a" mocked_app_config_schema = { "features": { "my_feature": { - "feature_default_value": "True", + "feature_default_value": True, "rules": [ { - "name": "tenant id equals 6 and username is a", - "rule_default_value": expected_value, + "rule_name": "tenant id equals 6 and username is a", + "value_when_applies": expected_value, "restrictions": [ { "action": ACTION.EQUALS.value, # this rule will match, it has multiple restrictions @@ -148,22 +149,22 @@ def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker, con }, value_if_missing=True, ) - assert str(toggle) == expected_value + assert toggle == expected_value # check a case when rule doesn't match and it has multiple restrictions, # different tenant id causes the rule to not match. # default value of the feature in this case is True def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker, config): - expected_val = "True" + expected_val = True mocked_app_config_schema = { "features": { "my_feature": { "feature_default_value": expected_val, "rules": [ { - "name": "tenant id equals 645654 and username is a", # rule will not match - "rule_default_value": "False", + "rule_name": "tenant id equals 645654 and username is a", # rule will not match + "value_when_applies": False, "restrictions": [ { "action": ACTION.EQUALS.value, @@ -185,14 +186,14 @@ def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker, toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "6", "username": "a"}, value_if_missing=False ) - assert str(toggle) == expected_val + assert toggle == expected_val # check rule match for multiple of action types def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multiple_restrictions(mocker, config): - expected_value_first_check = "True" - expected_value_second_check = "False" - expected_value_third_check = "False" + expected_value_first_check = True + expected_value_second_check = False + expected_value_third_check = False expected_value_fourth_case = False mocked_app_config_schema = { "features": { @@ -200,8 +201,8 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl "feature_default_value": expected_value_third_check, "rules": [ { - "name": "tenant id equals 6 and username startswith a", - "rule_default_value": expected_value_first_check, + "rule_name": "tenant id equals 6 and username startswith a", + "value_when_applies": expected_value_first_check, "restrictions": [ { "action": ACTION.EQUALS.value, @@ -216,8 +217,8 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl ], }, { - "name": "tenant id equals 4446 and username startswith a and endswith z", - "rule_default_value": expected_value_second_check, + "rule_name": "tenant id equals 4446 and username startswith a and endswith z", + "value_when_applies": expected_value_second_check, "restrictions": [ { "action": ACTION.EQUALS.value, @@ -248,21 +249,21 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl rules_context={"tenant_id": "6", "username": "abcd"}, value_if_missing=False, ) - assert str(toggle) == expected_value_first_check + assert toggle == expected_value_first_check # match second rule toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "4446", "username": "az"}, value_if_missing=False, ) - assert str(toggle) == expected_value_second_check + assert toggle == expected_value_second_check # match no rule toggle = conf_store.get_feature_toggle( feature_name="my_feature", rules_context={"tenant_id": "11114446", "username": "ab"}, value_if_missing=False, ) - assert str(toggle) == expected_value_third_check + assert toggle == expected_value_third_check # feature doesn't exist toggle = conf_store.get_feature_toggle( feature_name="my_fake_feature", @@ -281,8 +282,8 @@ def test_toggles_match_rule_with_contains_action(mocker, config): "feature_default_value": expected_value, "rules": [ { - "name": "tenant id equals 345345435", - "rule_default_value": True, + "rule_name": "tenant id equals 345345435", + "value_when_applies": True, "restrictions": [ { "action": ACTION.CONTAINS.value, @@ -312,8 +313,8 @@ def test_toggles_no_match_rule_with_contains_action(mocker, config): "feature_default_value": expected_value, "rules": [ { - "name": "tenant id equals 345345435", - "rule_default_value": True, + "rule_name": "tenant id equals 345345435", + "value_when_applies": True, "restrictions": [ { "action": ACTION.CONTAINS.value, diff --git a/tests/functional/feature_toggles/test_schema_validation.py b/tests/functional/feature_toggles/test_schema_validation.py new file mode 100644 index 0000000000..a68caa854d --- /dev/null +++ b/tests/functional/feature_toggles/test_schema_validation.py @@ -0,0 +1,264 @@ +import logging + +import pytest # noqa: F401 + +from aws_lambda_powertools.utilities.feature_toggles.exceptions import ConfigurationException +from aws_lambda_powertools.utilities.feature_toggles.schema import ( + ACTION, + FEATURE_DEFAULT_VAL_KEY, + FEATURES_KEY, + RESTRICTION_ACTION, + RESTRICTION_KEY, + RESTRICTION_VALUE, + RESTRICTIONS_KEY, + RULE_DEFAULT_VALUE, + RULE_NAME_KEY, + RULES_KEY, + SchemaValidator, +) + +logger = logging.getLogger(__name__) + + +def test_invalid_features_dict(): + schema = {} + # empty dict + validator = SchemaValidator(logger) + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + schema = [] + # invalid type + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + # invalid features key + schema = {FEATURES_KEY: []} + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + +def test_empty_features_not_fail(): + schema = {FEATURES_KEY: {}} + validator = SchemaValidator(logger) + validator.validate_json_schema(schema) + + +def test_invalid_feature_dict(): + # invalid feature type, not dict + schema = {FEATURES_KEY: {"my_feature": []}} + validator = SchemaValidator(logger) + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + # empty feature dict + schema = {FEATURES_KEY: {"my_feature": {}}} + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + # invalid FEATURE_DEFAULT_VAL_KEY type, not boolean + schema = {FEATURES_KEY: {"my_feature": {FEATURE_DEFAULT_VAL_KEY: "False"}}} + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + # invalid FEATURE_DEFAULT_VAL_KEY type, not boolean #2 + schema = {FEATURES_KEY: {"my_feature": {FEATURE_DEFAULT_VAL_KEY: 5}}} + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + # invalid rules type, not list + schema = {FEATURES_KEY: {"my_feature": {FEATURE_DEFAULT_VAL_KEY: False, RULES_KEY: "4"}}} + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + +def test_valid_feature_dict(): + # no rules list at all + schema = {FEATURES_KEY: {"my_feature": {FEATURE_DEFAULT_VAL_KEY: False}}} + validator = SchemaValidator(logger) + validator.validate_json_schema(schema) + + # empty rules list + schema = {FEATURES_KEY: {"my_feature": {FEATURE_DEFAULT_VAL_KEY: False, RULES_KEY: []}}} + validator.validate_json_schema(schema) + + +def test_invalid_rule(): + # rules list is not a list of dict + schema = { + FEATURES_KEY: { + "my_feature": { + FEATURE_DEFAULT_VAL_KEY: False, + RULES_KEY: [ + "a", + "b", + ], + } + } + } + validator = SchemaValidator(logger) + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + # rules RULE_DEFAULT_VALUE is not bool + schema = { + FEATURES_KEY: { + "my_feature": { + FEATURE_DEFAULT_VAL_KEY: False, + RULES_KEY: [ + { + RULE_NAME_KEY: "tenant id equals 345345435", + RULE_DEFAULT_VALUE: "False", + }, + ], + } + } + } + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + # missing restrictions list + schema = { + FEATURES_KEY: { + "my_feature": { + FEATURE_DEFAULT_VAL_KEY: False, + RULES_KEY: [ + { + RULE_NAME_KEY: "tenant id equals 345345435", + RULE_DEFAULT_VALUE: False, + }, + ], + } + } + } + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + # restriction list is empty + schema = { + FEATURES_KEY: { + "my_feature": { + FEATURE_DEFAULT_VAL_KEY: False, + RULES_KEY: [ + {RULE_NAME_KEY: "tenant id equals 345345435", RULE_DEFAULT_VALUE: False, RESTRICTIONS_KEY: []}, + ], + } + } + } + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + # restriction is invalid type, not list + schema = { + FEATURES_KEY: { + "my_feature": { + FEATURE_DEFAULT_VAL_KEY: False, + RULES_KEY: [ + {RULE_NAME_KEY: "tenant id equals 345345435", RULE_DEFAULT_VALUE: False, RESTRICTIONS_KEY: {}}, + ], + } + } + } + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + +def test_invalid_restriction(): + # invalid restriction action + schema = { + FEATURES_KEY: { + "my_feature": { + FEATURE_DEFAULT_VAL_KEY: False, + RULES_KEY: [ + { + RULE_NAME_KEY: "tenant id equals 345345435", + RULE_DEFAULT_VALUE: False, + RESTRICTIONS_KEY: {RESTRICTION_ACTION: "stuff", RESTRICTION_KEY: "a", RESTRICTION_VALUE: "a"}, + }, + ], + } + } + } + validator = SchemaValidator(logger) + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + # missing restriction key and value + schema = { + FEATURES_KEY: { + "my_feature": { + FEATURE_DEFAULT_VAL_KEY: False, + RULES_KEY: [ + { + RULE_NAME_KEY: "tenant id equals 345345435", + RULE_DEFAULT_VALUE: False, + RESTRICTIONS_KEY: {RESTRICTION_ACTION: ACTION.EQUALS.value}, + }, + ], + } + } + } + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + # invalid restriction key type, not string + schema = { + FEATURES_KEY: { + "my_feature": { + FEATURE_DEFAULT_VAL_KEY: False, + RULES_KEY: [ + { + RULE_NAME_KEY: "tenant id equals 345345435", + RULE_DEFAULT_VALUE: False, + RESTRICTIONS_KEY: { + RESTRICTION_ACTION: ACTION.EQUALS.value, + RESTRICTION_KEY: 5, + RESTRICTION_VALUE: "a", + }, + }, + ], + } + } + } + with pytest.raises(ConfigurationException): + validator.validate_json_schema(schema) + + +def test_valid_restriction_all_actions(): + validator = SchemaValidator(logger) + schema = { + FEATURES_KEY: { + "my_feature": { + FEATURE_DEFAULT_VAL_KEY: False, + RULES_KEY: [ + { + RULE_NAME_KEY: "tenant id equals 645654 and username is a", + RULE_DEFAULT_VALUE: True, + RESTRICTIONS_KEY: [ + { + RESTRICTION_ACTION: ACTION.EQUALS.value, + RESTRICTION_KEY: "tenant_id", + RESTRICTION_VALUE: "645654", + }, + { + RESTRICTION_ACTION: ACTION.STARTSWITH.value, + RESTRICTION_KEY: "username", + RESTRICTION_VALUE: "a", + }, + { + RESTRICTION_ACTION: ACTION.ENDSWITH.value, + RESTRICTION_KEY: "username", + RESTRICTION_VALUE: "a", + }, + { + RESTRICTION_ACTION: ACTION.CONTAINS.value, + RESTRICTION_KEY: "username", + RESTRICTION_VALUE: ["a", "b"], + }, + ], + }, + ], + } + }, + } + validator.validate_json_schema(schema) From 2a72a92d05fe12ed3ff4b59a4d3126951045431d Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Sat, 3 Jul 2021 12:16:27 +0300 Subject: [PATCH 08/12] added list all enabled feature toggles --- .../feature_toggles/configuration_store.py | 91 ++++++++++++------ .../feature_toggles/test_feature_toggles.py | 93 ++++++++++++++++++- 2 files changed, 153 insertions(+), 31 deletions(-) diff --git a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py index 1413492008..827b7b908f 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py +++ b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py @@ -49,6 +49,29 @@ def _match_by_action(self, action: str, restriction_value: Any, context_value: A self._logger.error(f"caught exception while matching action, action={action}, exception={str(exc)}") return False + def _is_rule_matched(self, feature_name: str, rule: Dict[str, Any], rules_context: Dict[str, Any]) -> bool: + rule_name = rule.get(schema.RULE_NAME_KEY, "") + rule_default_value = rule.get(schema.RULE_DEFAULT_VALUE) + restrictions: Dict[str, str] = rule.get(schema.RESTRICTIONS_KEY) + + for restriction in restrictions: + context_value = rules_context.get(restriction.get(schema.RESTRICTION_KEY)) + if not self._match_by_action( + restriction.get(schema.RESTRICTION_ACTION), + restriction.get(schema.RESTRICTION_VALUE), + context_value, + ): + logger.debug( + f"rule did not match action, rule_name={rule_name}, rule_default_value={rule_default_value}, feature_name={feature_name}, context_value={str(context_value)}" # noqa: E501 + ) + # context doesn't match restriction + return False + # if we got here, all restrictions match + logger.debug( + f"rule matched, rule_name={rule_name}, rule_default_value={rule_default_value}, feature_name={feature_name}" # noqa: E501 + ) + return True + def _handle_rules( self, *, @@ -58,34 +81,14 @@ def _handle_rules( rules: List[Dict[str, Any]], ) -> bool: for rule in rules: - rule_name = rule.get(schema.RULE_NAME_KEY, "") rule_default_value = rule.get(schema.RULE_DEFAULT_VALUE) - is_match = True - restrictions: Dict[str, str] = rule.get(schema.RESTRICTIONS_KEY) - - for restriction in restrictions: - context_value = rules_context.get(restriction.get(schema.RESTRICTION_KEY)) - if not self._match_by_action( - restriction.get(schema.RESTRICTION_ACTION), - restriction.get(schema.RESTRICTION_VALUE), - context_value, - ): - logger.debug( - f"rule did not match action, rule_name={rule_name}, rule_default_value={rule_default_value}, feature_name={feature_name}, context_value={str(context_value)}" # noqa: E501 - ) - is_match = False # rules doesn't match restriction - break - # if we got here, all restrictions match - if is_match: - logger.debug( - f"rule matched, rule_name={rule_name}, rule_default_value={rule_default_value}, feature_name={feature_name}" # noqa: E501 - ) + if self._is_rule_matched(feature_name, rule, rules_context): return rule_default_value - # no rule matched, return default value of feature - logger.debug( - f"no rule matched, returning default value of feature, feature_default_value={feature_default_value}, feature_name={feature_name}" # noqa: E501 - ) - return feature_default_value + # no rule matched, return default value of feature + logger.debug( + f"no rule matched, returning default value of feature, feature_default_value={feature_default_value}, feature_name={feature_name}" # noqa: E501 + ) + return feature_default_value def get_configuration(self) -> Dict[str, Any]: """Get configuration string from AWs AppConfig and returned the parsed JSON dictionary @@ -162,3 +165,39 @@ def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any] feature_default_value=feature_default_value, rules=rules_list, ) + + def get_all_enabled_feature_toggles(self, *, rules_context: Dict[str, Any]) -> List[str]: + """Get all enabled feature toggles while also taking into account rule_context (when a feature has defined rules) + + Args: + rules_context (Dict[str, Any]): dict of attributes that you would like to match the rules + against, can be {'tenant_id: 'X', 'username':' 'Y', 'region': 'Z'} etc. + + Returns: + List[str]: a list of all features name that are enabled by also taking into account + rule_context (when a feature has defined rules) + """ + try: + toggles_dict: Dict[str, Any] = self.get_configuration() + except ConfigurationException: + logger.error("unable to get feature toggles JSON") # noqa: E501 + return [] + ret_list = [] + features: Dict[str, Any] = toggles_dict.get(schema.FEATURES_KEY, {}) + for feature_name, feature_dict_def in features.items(): + rules_list = feature_dict_def.get(schema.RULES_KEY, []) + feature_default_value = feature_dict_def.get(schema.FEATURE_DEFAULT_VAL_KEY) + if feature_default_value and not rules_list: + self._logger.debug( + f"feature is enabled by default and has no defined rules, feature_name={feature_name}" + ) + ret_list.append(feature_name) + elif self._handle_rules( + feature_name=feature_name, + rules_context=rules_context, + feature_default_value=feature_default_value, + rules=rules_list, + ): + self._logger.debug(f"feature's calculated value is True, feature_name={feature_name}") + ret_list.append(feature_name) + return ret_list diff --git a/tests/functional/feature_toggles/test_feature_toggles.py b/tests/functional/feature_toggles/test_feature_toggles.py index 8289451b0c..6845e1c3e5 100644 --- a/tests/functional/feature_toggles/test_feature_toggles.py +++ b/tests/functional/feature_toggles/test_feature_toggles.py @@ -1,4 +1,4 @@ -from typing import Dict +from typing import Dict, List import pytest # noqa: F401 from botocore.config import Config @@ -279,11 +279,11 @@ def test_toggles_match_rule_with_contains_action(mocker, config): mocked_app_config_schema = { "features": { "my_feature": { - "feature_default_value": expected_value, + "feature_default_value": False, "rules": [ { - "rule_name": "tenant id equals 345345435", - "value_when_applies": True, + "rule_name": "tenant id is contained in [6,2] ", + "value_when_applies": expected_value, "restrictions": [ { "action": ACTION.CONTAINS.value, @@ -313,7 +313,7 @@ def test_toggles_no_match_rule_with_contains_action(mocker, config): "feature_default_value": expected_value, "rules": [ { - "rule_name": "tenant id equals 345345435", + "rule_name": "tenant id is contained in [6,2] ", "value_when_applies": True, "restrictions": [ { @@ -334,3 +334,86 @@ def test_toggles_no_match_rule_with_contains_action(mocker, config): value_if_missing=False, ) assert toggle == expected_value + + +def test_multiple_features_enabled(mocker, config): + expected_value = ["my_feature", "my_feature2"] + mocked_app_config_schema = { + "features": { + "my_feature": { + "feature_default_value": False, + "rules": [ + { + "rule_name": "tenant id is contained in [6,2] ", + "value_when_applies": True, + "restrictions": [ + { + "action": ACTION.CONTAINS.value, + "key": "tenant_id", + "value": ["6", "2"], + } + ], + }, + ], + }, + "my_feature2": { + "feature_default_value": True, + }, + }, + } + conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) + enabled_list: List[str] = conf_store.get_all_enabled_feature_toggles( + rules_context={"tenant_id": "6", "username": "a"} + ) + assert enabled_list == expected_value + + +def test_multiple_features_only_some_enabled(mocker, config): + expected_value = ["my_feature", "my_feature2", "my_feature4"] + mocked_app_config_schema = { + "features": { + "my_feature": { # rule will match here, feature is enabled due to rule match + "feature_default_value": False, + "rules": [ + { + "rule_name": "tenant id is contained in [6,2] ", + "value_when_applies": True, + "restrictions": [ + { + "action": ACTION.CONTAINS.value, + "key": "tenant_id", + "value": ["6", "2"], + } + ], + }, + ], + }, + "my_feature2": { + "feature_default_value": True, + }, + "my_feature3": { + "feature_default_value": False, + }, + "my_feature4": { # rule will not match here, feature is enabled by default + "feature_default_value": True, + "rules": [ + { + "rule_name": "tenant id equals 7", + "value_when_applies": False, + "restrictions": [ + { + "action": ACTION.EQUALS.value, + "key": "tenant_id", + "value": "7", + } + ], + }, + ], + }, + }, + } + conf_store = init_configuration_store(mocker, mocked_app_config_schema, config) + enabled_list: List[str] = conf_store.get_all_enabled_feature_toggles( + rules_context={"tenant_id": "6", "username": "a"} + ) + assert enabled_list == expected_value From dffbe7c3be570f3c3ba95800154512068aec54f2 Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Sun, 4 Jul 2021 20:52:22 +0300 Subject: [PATCH 09/12] optional context --- .../feature_toggles/configuration_store.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py index 827b7b908f..7ed92e39d1 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py +++ b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py @@ -114,13 +114,15 @@ def get_configuration(self) -> Dict[str, Any]: self._schema_validator.validate_json_schema(schema) return schema - def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any], value_if_missing: bool) -> bool: + def get_feature_toggle( + self, *, feature_name: str, rules_context: Optional[Dict[str, Any]] = None, value_if_missing: bool + ) -> bool: """get a feature toggle boolean value. Value is calculated according to a set of rules and conditions. see below for explanation. Args: feature_name (str): feature name that you wish to fetch - rules_context (Dict[str, Any]): dict of attributes that you would like to match the rules + rules_context (Optional[Dict[str, Any]]): dict of attributes that you would like to match the rules against, can be {'tenant_id: 'X', 'username':' 'Y', 'region': 'Z'} etc. value_if_missing (bool): this will be the returned value in case the feature toggle doesn't exist in the schema or there has been an error while fetching the @@ -134,6 +136,9 @@ def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any] the defined feature 3. feature exists and a rule matches -> rule_default_value of rule is returned """ + if rules_context is None: + rules_context = {} + try: toggles_dict: Dict[str, Any] = self.get_configuration() except ConfigurationException: @@ -166,17 +171,19 @@ def get_feature_toggle(self, *, feature_name: str, rules_context: Dict[str, Any] rules=rules_list, ) - def get_all_enabled_feature_toggles(self, *, rules_context: Dict[str, Any]) -> List[str]: + def get_all_enabled_feature_toggles(self, *, rules_context: Optional[Dict[str, Any]] = None) -> List[str]: """Get all enabled feature toggles while also taking into account rule_context (when a feature has defined rules) Args: - rules_context (Dict[str, Any]): dict of attributes that you would like to match the rules + rules_context (Optional[Dict[str, Any]]): dict of attributes that you would like to match the rules against, can be {'tenant_id: 'X', 'username':' 'Y', 'region': 'Z'} etc. Returns: List[str]: a list of all features name that are enabled by also taking into account rule_context (when a feature has defined rules) """ + if rules_context is None: + rules_context = {} try: toggles_dict: Dict[str, Any] = self.get_configuration() except ConfigurationException: From 6ec9a8fbdf63775d434deeca2f733210153a8d89 Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Mon, 5 Jul 2021 14:27:12 +0300 Subject: [PATCH 10/12] rename restriction to condition --- .../feature_toggles/configuration_store.py | 18 ++--- .../utilities/feature_toggles/schema.py | 34 +++++----- .../feature_toggles/test_feature_toggles.py | 38 +++++------ .../feature_toggles/test_schema_validation.py | 66 +++++++++---------- 4 files changed, 78 insertions(+), 78 deletions(-) diff --git a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py index 7ed92e39d1..30540c1a8d 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py +++ b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py @@ -32,7 +32,7 @@ def __init__( self._schema_validator = schema.SchemaValidator(self._logger) self._conf_store = AppConfigProvider(environment=environment, application=service, config=config) - def _match_by_action(self, action: str, restriction_value: Any, context_value: Any) -> bool: + def _match_by_action(self, action: str, CONDITION_VALUE: Any, context_value: Any) -> bool: if not context_value: return False mapping_by_action = { @@ -44,7 +44,7 @@ def _match_by_action(self, action: str, restriction_value: Any, context_value: A try: func = mapping_by_action.get(action, lambda a, b: False) - return func(context_value, restriction_value) + return func(context_value, CONDITION_VALUE) except Exception as exc: self._logger.error(f"caught exception while matching action, action={action}, exception={str(exc)}") return False @@ -52,21 +52,21 @@ def _match_by_action(self, action: str, restriction_value: Any, context_value: A def _is_rule_matched(self, feature_name: str, rule: Dict[str, Any], rules_context: Dict[str, Any]) -> bool: rule_name = rule.get(schema.RULE_NAME_KEY, "") rule_default_value = rule.get(schema.RULE_DEFAULT_VALUE) - restrictions: Dict[str, str] = rule.get(schema.RESTRICTIONS_KEY) + conditions: Dict[str, str] = rule.get(schema.CONDITIONS_KEY) - for restriction in restrictions: - context_value = rules_context.get(restriction.get(schema.RESTRICTION_KEY)) + for condition in conditions: + context_value = rules_context.get(condition.get(schema.CONDITION_KEY)) if not self._match_by_action( - restriction.get(schema.RESTRICTION_ACTION), - restriction.get(schema.RESTRICTION_VALUE), + condition.get(schema.CONDITION_ACTION), + condition.get(schema.CONDITION_VALUE), context_value, ): logger.debug( f"rule did not match action, rule_name={rule_name}, rule_default_value={rule_default_value}, feature_name={feature_name}, context_value={str(context_value)}" # noqa: E501 ) - # context doesn't match restriction + # context doesn't match condition return False - # if we got here, all restrictions match + # if we got here, all conditions match logger.debug( f"rule matched, rule_name={rule_name}, rule_default_value={rule_default_value}, feature_name={feature_name}" # noqa: E501 ) diff --git a/aws_lambda_powertools/utilities/feature_toggles/schema.py b/aws_lambda_powertools/utilities/feature_toggles/schema.py index 35500da176..58e75fabfc 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/schema.py +++ b/aws_lambda_powertools/utilities/feature_toggles/schema.py @@ -6,12 +6,12 @@ FEATURES_KEY = "features" RULES_KEY = "rules" FEATURE_DEFAULT_VAL_KEY = "feature_default_value" -RESTRICTIONS_KEY = "restrictions" +CONDITIONS_KEY = "conditions" RULE_NAME_KEY = "rule_name" RULE_DEFAULT_VALUE = "value_when_applies" -RESTRICTION_KEY = "key" -RESTRICTION_VALUE = "value" -RESTRICTION_ACTION = "action" +CONDITION_KEY = "key" +CONDITION_VALUE = "value" +CONDITION_ACTION = "action" class ACTION(str, Enum): @@ -29,18 +29,18 @@ def _raise_conf_exc(self, error_str: str) -> None: self._logger.error(error_str) raise ConfigurationException(error_str) - def _validate_restriction(self, rule_name: str, restriction: Dict[str, str]) -> None: - if not restriction or not isinstance(restriction, dict): - self._raise_conf_exc(f"invalid restriction type, not a dictionary, rule_name={rule_name}") - action = restriction.get(RESTRICTION_ACTION, "") + def _validate_condition(self, rule_name: str, condition: Dict[str, str]) -> None: + if not condition or not isinstance(condition, dict): + self._raise_conf_exc(f"invalid condition type, not a dictionary, rule_name={rule_name}") + action = condition.get(CONDITION_ACTION, "") if action not in [ACTION.EQUALS.value, ACTION.STARTSWITH.value, ACTION.ENDSWITH.value, ACTION.CONTAINS.value]: self._raise_conf_exc(f"invalid action value, rule_name={rule_name}, action={action}") - key = restriction.get(RESTRICTION_KEY, "") + key = condition.get(CONDITION_KEY, "") if not key or not isinstance(key, str): self._raise_conf_exc(f"invalid key value, key has to be a non empty string, rule_name={rule_name}") - value = restriction.get(RESTRICTION_VALUE, "") + value = condition.get(CONDITION_VALUE, "") if not value: - self._raise_conf_exc(f"missing restriction value, rule_name={rule_name}") + self._raise_conf_exc(f"missing condition value, rule_name={rule_name}") def _validate_rule(self, feature_name: str, rule: Dict[str, Any]) -> None: if not rule or not isinstance(rule, dict): @@ -51,12 +51,12 @@ def _validate_rule(self, feature_name: str, rule: Dict[str, Any]) -> None: rule_default_value = rule.get(RULE_DEFAULT_VALUE) if rule_default_value is None or not isinstance(rule_default_value, bool): self._raise_conf_exc(f"invalid rule_default_value, rule_name={rule_name}") - restrictions = rule.get(RESTRICTIONS_KEY, {}) - if not restrictions or not isinstance(restrictions, list): - self._raise_conf_exc(f"invalid restrictions, rule_name={rule_name}") - # validate restrictions - for restriction in restrictions: - self._validate_restriction(rule_name, restriction) + conditions = rule.get(CONDITIONS_KEY, {}) + if not conditions or not isinstance(conditions, list): + self._raise_conf_exc(f"invalid condition, rule_name={rule_name}") + # validate conditions + for condition in conditions: + self._validate_condition(rule_name, condition) def _validate_feature(self, feature_name: str, feature_dict_def: Dict[str, Any]) -> None: if not feature_dict_def or not isinstance(feature_dict_def, dict): diff --git a/tests/functional/feature_toggles/test_feature_toggles.py b/tests/functional/feature_toggles/test_feature_toggles.py index 6845e1c3e5..4c79c42abf 100644 --- a/tests/functional/feature_toggles/test_feature_toggles.py +++ b/tests/functional/feature_toggles/test_feature_toggles.py @@ -37,7 +37,7 @@ def test_toggles_rule_does_not_match(mocker, config): { "rule_name": "tenant id equals 345345435", "value_when_applies": False, - "restrictions": [ + "conditions": [ { "action": ACTION.EQUALS.value, "key": "tenant_id", @@ -57,7 +57,7 @@ def test_toggles_rule_does_not_match(mocker, config): # this test checks that if you try to get a feature that doesn't exist in the schema, # you get the default value of False that was sent to the get_feature_toggle API -def test_toggles_no_restrictions_feature_does_not_exist(mocker, config): +def test_toggles_no_conditions_feature_does_not_exist(mocker, config): expected_value = False mocked_app_config_schema = {"features": {"my_fake_feature": {"feature_default_value": True}}} @@ -79,7 +79,7 @@ def test_toggles_no_rules(mocker, config): # check a case where the feature exists but the rule doesn't match so we revert to the default value of the feature -def test_toggles_restrictions_no_match(mocker, config): +def test_toggles_conditions_no_match(mocker, config): expected_value = True mocked_app_config_schema = { "features": { @@ -89,7 +89,7 @@ def test_toggles_restrictions_no_match(mocker, config): { "rule_name": "tenant id equals 345345435", "value_when_applies": False, - "restrictions": [ + "conditions": [ { "action": ACTION.EQUALS.value, "key": "tenant_id", @@ -110,8 +110,8 @@ def test_toggles_restrictions_no_match(mocker, config): assert toggle == expected_value -# check that a rule can match when it has multiple restrictions, see rule name for further explanation -def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker, config): +# check that a rule can match when it has multiple conditions, see rule name for further explanation +def test_toggles_conditions_rule_match_equal_multiple_conditions(mocker, config): expected_value = False tenant_id_val = "6" username_val = "a" @@ -123,9 +123,9 @@ def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker, con { "rule_name": "tenant id equals 6 and username is a", "value_when_applies": expected_value, - "restrictions": [ + "conditions": [ { - "action": ACTION.EQUALS.value, # this rule will match, it has multiple restrictions + "action": ACTION.EQUALS.value, # this rule will match, it has multiple conditions "key": "tenant_id", "value": tenant_id_val, }, @@ -152,10 +152,10 @@ def test_toggles_restrictions_rule_match_equal_multiple_restrictions(mocker, con assert toggle == expected_value -# check a case when rule doesn't match and it has multiple restrictions, +# check a case when rule doesn't match and it has multiple conditions, # different tenant id causes the rule to not match. # default value of the feature in this case is True -def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker, config): +def test_toggles_conditions_no_rule_match_equal_multiple_conditions(mocker, config): expected_val = True mocked_app_config_schema = { "features": { @@ -165,7 +165,7 @@ def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker, { "rule_name": "tenant id equals 645654 and username is a", # rule will not match "value_when_applies": False, - "restrictions": [ + "conditions": [ { "action": ACTION.EQUALS.value, "key": "tenant_id", @@ -190,7 +190,7 @@ def test_toggles_restrictions_no_rule_match_equal_multiple_restrictions(mocker, # check rule match for multiple of action types -def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multiple_restrictions(mocker, config): +def test_toggles_conditions_rule_match_multiple_actions_multiple_rules_multiple_conditions(mocker, config): expected_value_first_check = True expected_value_second_check = False expected_value_third_check = False @@ -203,7 +203,7 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl { "rule_name": "tenant id equals 6 and username startswith a", "value_when_applies": expected_value_first_check, - "restrictions": [ + "conditions": [ { "action": ACTION.EQUALS.value, "key": "tenant_id", @@ -219,7 +219,7 @@ def test_toggles_restrictions_rule_match_multiple_actions_multiple_rules_multipl { "rule_name": "tenant id equals 4446 and username startswith a and endswith z", "value_when_applies": expected_value_second_check, - "restrictions": [ + "conditions": [ { "action": ACTION.EQUALS.value, "key": "tenant_id", @@ -284,7 +284,7 @@ def test_toggles_match_rule_with_contains_action(mocker, config): { "rule_name": "tenant id is contained in [6,2] ", "value_when_applies": expected_value, - "restrictions": [ + "conditions": [ { "action": ACTION.CONTAINS.value, "key": "tenant_id", @@ -315,7 +315,7 @@ def test_toggles_no_match_rule_with_contains_action(mocker, config): { "rule_name": "tenant id is contained in [6,2] ", "value_when_applies": True, - "restrictions": [ + "conditions": [ { "action": ACTION.CONTAINS.value, "key": "tenant_id", @@ -346,7 +346,7 @@ def test_multiple_features_enabled(mocker, config): { "rule_name": "tenant id is contained in [6,2] ", "value_when_applies": True, - "restrictions": [ + "conditions": [ { "action": ACTION.CONTAINS.value, "key": "tenant_id", @@ -378,7 +378,7 @@ def test_multiple_features_only_some_enabled(mocker, config): { "rule_name": "tenant id is contained in [6,2] ", "value_when_applies": True, - "restrictions": [ + "conditions": [ { "action": ACTION.CONTAINS.value, "key": "tenant_id", @@ -400,7 +400,7 @@ def test_multiple_features_only_some_enabled(mocker, config): { "rule_name": "tenant id equals 7", "value_when_applies": False, - "restrictions": [ + "conditions": [ { "action": ACTION.EQUALS.value, "key": "tenant_id", diff --git a/tests/functional/feature_toggles/test_schema_validation.py b/tests/functional/feature_toggles/test_schema_validation.py index a68caa854d..9febae65ba 100644 --- a/tests/functional/feature_toggles/test_schema_validation.py +++ b/tests/functional/feature_toggles/test_schema_validation.py @@ -7,10 +7,10 @@ ACTION, FEATURE_DEFAULT_VAL_KEY, FEATURES_KEY, - RESTRICTION_ACTION, - RESTRICTION_KEY, - RESTRICTION_VALUE, - RESTRICTIONS_KEY, + CONDITION_ACTION, + CONDITION_KEY, + CONDITION_VALUE, + CONDITIONS_KEY, RULE_DEFAULT_VALUE, RULE_NAME_KEY, RULES_KEY, @@ -117,7 +117,7 @@ def test_invalid_rule(): with pytest.raises(ConfigurationException): validator.validate_json_schema(schema) - # missing restrictions list + # missing conditions list schema = { FEATURES_KEY: { "my_feature": { @@ -134,13 +134,13 @@ def test_invalid_rule(): with pytest.raises(ConfigurationException): validator.validate_json_schema(schema) - # restriction list is empty + # condition list is empty schema = { FEATURES_KEY: { "my_feature": { FEATURE_DEFAULT_VAL_KEY: False, RULES_KEY: [ - {RULE_NAME_KEY: "tenant id equals 345345435", RULE_DEFAULT_VALUE: False, RESTRICTIONS_KEY: []}, + {RULE_NAME_KEY: "tenant id equals 345345435", RULE_DEFAULT_VALUE: False, CONDITIONS_KEY: []}, ], } } @@ -148,13 +148,13 @@ def test_invalid_rule(): with pytest.raises(ConfigurationException): validator.validate_json_schema(schema) - # restriction is invalid type, not list + # condition is invalid type, not list schema = { FEATURES_KEY: { "my_feature": { FEATURE_DEFAULT_VAL_KEY: False, RULES_KEY: [ - {RULE_NAME_KEY: "tenant id equals 345345435", RULE_DEFAULT_VALUE: False, RESTRICTIONS_KEY: {}}, + {RULE_NAME_KEY: "tenant id equals 345345435", RULE_DEFAULT_VALUE: False, CONDITIONS_KEY: {}}, ], } } @@ -163,8 +163,8 @@ def test_invalid_rule(): validator.validate_json_schema(schema) -def test_invalid_restriction(): - # invalid restriction action +def test_invalid_condition(): + # invalid condition action schema = { FEATURES_KEY: { "my_feature": { @@ -173,7 +173,7 @@ def test_invalid_restriction(): { RULE_NAME_KEY: "tenant id equals 345345435", RULE_DEFAULT_VALUE: False, - RESTRICTIONS_KEY: {RESTRICTION_ACTION: "stuff", RESTRICTION_KEY: "a", RESTRICTION_VALUE: "a"}, + CONDITIONS_KEY: {CONDITION_ACTION: "stuff", CONDITION_KEY: "a", CONDITION_VALUE: "a"}, }, ], } @@ -183,7 +183,7 @@ def test_invalid_restriction(): with pytest.raises(ConfigurationException): validator.validate_json_schema(schema) - # missing restriction key and value + # missing condition key and value schema = { FEATURES_KEY: { "my_feature": { @@ -192,7 +192,7 @@ def test_invalid_restriction(): { RULE_NAME_KEY: "tenant id equals 345345435", RULE_DEFAULT_VALUE: False, - RESTRICTIONS_KEY: {RESTRICTION_ACTION: ACTION.EQUALS.value}, + CONDITIONS_KEY: {CONDITION_ACTION: ACTION.EQUALS.value}, }, ], } @@ -201,7 +201,7 @@ def test_invalid_restriction(): with pytest.raises(ConfigurationException): validator.validate_json_schema(schema) - # invalid restriction key type, not string + # invalid condition key type, not string schema = { FEATURES_KEY: { "my_feature": { @@ -210,10 +210,10 @@ def test_invalid_restriction(): { RULE_NAME_KEY: "tenant id equals 345345435", RULE_DEFAULT_VALUE: False, - RESTRICTIONS_KEY: { - RESTRICTION_ACTION: ACTION.EQUALS.value, - RESTRICTION_KEY: 5, - RESTRICTION_VALUE: "a", + CONDITIONS_KEY: { + CONDITION_ACTION: ACTION.EQUALS.value, + CONDITION_KEY: 5, + CONDITION_VALUE: "a", }, }, ], @@ -224,7 +224,7 @@ def test_invalid_restriction(): validator.validate_json_schema(schema) -def test_valid_restriction_all_actions(): +def test_valid_condition_all_actions(): validator = SchemaValidator(logger) schema = { FEATURES_KEY: { @@ -234,26 +234,26 @@ def test_valid_restriction_all_actions(): { RULE_NAME_KEY: "tenant id equals 645654 and username is a", RULE_DEFAULT_VALUE: True, - RESTRICTIONS_KEY: [ + CONDITIONS_KEY: [ { - RESTRICTION_ACTION: ACTION.EQUALS.value, - RESTRICTION_KEY: "tenant_id", - RESTRICTION_VALUE: "645654", + CONDITION_ACTION: ACTION.EQUALS.value, + CONDITION_KEY: "tenant_id", + CONDITION_VALUE: "645654", }, { - RESTRICTION_ACTION: ACTION.STARTSWITH.value, - RESTRICTION_KEY: "username", - RESTRICTION_VALUE: "a", + CONDITION_ACTION: ACTION.STARTSWITH.value, + CONDITION_KEY: "username", + CONDITION_VALUE: "a", }, { - RESTRICTION_ACTION: ACTION.ENDSWITH.value, - RESTRICTION_KEY: "username", - RESTRICTION_VALUE: "a", + CONDITION_ACTION: ACTION.ENDSWITH.value, + CONDITION_KEY: "username", + CONDITION_VALUE: "a", }, { - RESTRICTION_ACTION: ACTION.CONTAINS.value, - RESTRICTION_KEY: "username", - RESTRICTION_VALUE: ["a", "b"], + CONDITION_ACTION: ACTION.CONTAINS.value, + CONDITION_KEY: "username", + CONDITION_VALUE: ["a", "b"], }, ], }, From b0c9daa24ccde9d426bccb335855d67462599af7 Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Mon, 5 Jul 2021 14:28:00 +0300 Subject: [PATCH 11/12] rename --- .../utilities/feature_toggles/configuration_store.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py index 30540c1a8d..b285f43535 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py +++ b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py @@ -32,7 +32,7 @@ def __init__( self._schema_validator = schema.SchemaValidator(self._logger) self._conf_store = AppConfigProvider(environment=environment, application=service, config=config) - def _match_by_action(self, action: str, CONDITION_VALUE: Any, context_value: Any) -> bool: + def _match_by_action(self, action: str, condition_value: Any, context_value: Any) -> bool: if not context_value: return False mapping_by_action = { @@ -44,7 +44,7 @@ def _match_by_action(self, action: str, CONDITION_VALUE: Any, context_value: Any try: func = mapping_by_action.get(action, lambda a, b: False) - return func(context_value, CONDITION_VALUE) + return func(context_value, condition_value) except Exception as exc: self._logger.error(f"caught exception while matching action, action={action}, exception={str(exc)}") return False From 2679cb91d7663694847dc64ee6899a288bcfe024 Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Thu, 8 Jul 2021 17:36:52 +0300 Subject: [PATCH 12/12] added abstract schema fetcher class --- .../utilities/feature_toggles/__init__.py | 4 ++ .../feature_toggles/appconfig_fetcher.py | 57 +++++++++++++++++++ .../feature_toggles/configuration_store.py | 37 +++--------- .../utilities/feature_toggles/exceptions.py | 2 +- .../feature_toggles/schema_fetcher.py | 20 +++++++ .../feature_toggles/test_feature_toggles.py | 7 ++- .../feature_toggles/test_schema_validation.py | 4 +- 7 files changed, 98 insertions(+), 33 deletions(-) create mode 100644 aws_lambda_powertools/utilities/feature_toggles/appconfig_fetcher.py create mode 100644 aws_lambda_powertools/utilities/feature_toggles/schema_fetcher.py diff --git a/aws_lambda_powertools/utilities/feature_toggles/__init__.py b/aws_lambda_powertools/utilities/feature_toggles/__init__.py index be5b024539..378f7e23f4 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/__init__.py +++ b/aws_lambda_powertools/utilities/feature_toggles/__init__.py @@ -1,12 +1,16 @@ """Advanced feature toggles utility """ +from .appconfig_fetcher import AppConfigFetcher from .configuration_store import ConfigurationStore from .exceptions import ConfigurationException from .schema import ACTION, SchemaValidator +from .schema_fetcher import SchemaFetcher __all__ = [ "ConfigurationException", "ConfigurationStore", "ACTION", "SchemaValidator", + "AppConfigFetcher", + "SchemaFetcher", ] diff --git a/aws_lambda_powertools/utilities/feature_toggles/appconfig_fetcher.py b/aws_lambda_powertools/utilities/feature_toggles/appconfig_fetcher.py new file mode 100644 index 0000000000..177d4ed0ae --- /dev/null +++ b/aws_lambda_powertools/utilities/feature_toggles/appconfig_fetcher.py @@ -0,0 +1,57 @@ +import logging +from typing import Any, Dict, Optional + +from botocore.config import Config + +from aws_lambda_powertools.utilities.parameters import AppConfigProvider, GetParameterError, TransformParameterError + +from .exceptions import ConfigurationException +from .schema_fetcher import SchemaFetcher + +logger = logging.getLogger(__name__) + + +TRANSFORM_TYPE = "json" + + +class AppConfigFetcher(SchemaFetcher): + def __init__( + self, + environment: str, + service: str, + configuration_name: str, + cache_seconds: int, + config: Optional[Config] = None, + ): + """This class fetches JSON schemas from AWS AppConfig + + Args: + environment (str): what appconfig environment to use 'dev/test' etc. + service (str): what service name to use from the supplied environment + configuration_name (str): what configuration to take from the environment & service combination + cache_seconds (int): cache expiration time, how often to call AppConfig to fetch latest configuration + config (Optional[Config]): boto3 client configuration + """ + super().__init__(configuration_name, cache_seconds) + self._logger = logger + self._conf_store = AppConfigProvider(environment=environment, application=service, config=config) + + def get_json_configuration(self) -> Dict[str, Any]: + """Get configuration string from AWs AppConfig and return the parsed JSON dictionary + + Raises: + ConfigurationException: Any validation error or appconfig error that can occur + + Returns: + Dict[str, Any]: parsed JSON dictionary + """ + try: + return self._conf_store.get( + name=self.configuration_name, + transform=TRANSFORM_TYPE, + max_age=self._cache_seconds, + ) # parse result conf as JSON, keep in cache for self.max_age seconds + except (GetParameterError, TransformParameterError) as exc: + error_str = f"unable to get AWS AppConfig configuration file, exception={str(exc)}" + self._logger.error(error_str) + raise ConfigurationException(error_str) diff --git a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py index b285f43535..e540447737 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py +++ b/aws_lambda_powertools/utilities/feature_toggles/configuration_store.py @@ -1,36 +1,23 @@ -# pylint: disable=no-name-in-module,line-too-long import logging from typing import Any, Dict, List, Optional -from botocore.config import Config - -from aws_lambda_powertools.utilities.parameters import AppConfigProvider, GetParameterError, TransformParameterError - from . import schema from .exceptions import ConfigurationException - -TRANSFORM_TYPE = "json" +from .schema_fetcher import SchemaFetcher logger = logging.getLogger(__name__) class ConfigurationStore: - def __init__( - self, environment: str, service: str, conf_name: str, cache_seconds: int, config: Optional[Config] = None - ): + def __init__(self, schema_fetcher: SchemaFetcher): """constructor Args: - environment (str): what appconfig environment to use 'dev/test' etc. - service (str): what service name to use from the supplied environment - conf_name (str): what configuration to take from the environment & service combination - cache_seconds (int): cache expiration time, how often to call AppConfig to fetch latest configuration + schema_fetcher (SchemaFetcher): A schema JSON fetcher, can be AWS AppConfig, Hashicorp Consul etc. """ - self._cache_seconds = cache_seconds self._logger = logger - self._conf_name = conf_name + self._schema_fetcher = schema_fetcher self._schema_validator = schema.SchemaValidator(self._logger) - self._conf_store = AppConfigProvider(environment=environment, application=service, config=config) def _match_by_action(self, action: str, condition_value: Any, context_value: Any) -> bool: if not context_value: @@ -99,17 +86,11 @@ def get_configuration(self) -> Dict[str, Any]: Returns: Dict[str, Any]: parsed JSON dictionary """ - try: - schema = self._conf_store.get( - name=self._conf_name, - transform=TRANSFORM_TYPE, - max_age=self._cache_seconds, - ) # parse result conf as JSON, keep in cache for self.max_age seconds - except (GetParameterError, TransformParameterError) as exc: - error_str = f"unable to get AWS AppConfig configuration file, exception={str(exc)}" - self._logger.error(error_str) - raise ConfigurationException(error_str) - + schema: Dict[ + str, Any + ] = ( + self._schema_fetcher.get_json_configuration() + ) # parse result conf as JSON, keep in cache for self.max_age seconds # validate schema self._schema_validator.validate_json_schema(schema) return schema diff --git a/aws_lambda_powertools/utilities/feature_toggles/exceptions.py b/aws_lambda_powertools/utilities/feature_toggles/exceptions.py index 6fed03d805..9bbb5f200b 100644 --- a/aws_lambda_powertools/utilities/feature_toggles/exceptions.py +++ b/aws_lambda_powertools/utilities/feature_toggles/exceptions.py @@ -1,2 +1,2 @@ class ConfigurationException(Exception): - """When a a configuration store raises an exception on schema retrieval or parsing""" + """When a a configuration store raises an exception on config retrieval or parsing""" diff --git a/aws_lambda_powertools/utilities/feature_toggles/schema_fetcher.py b/aws_lambda_powertools/utilities/feature_toggles/schema_fetcher.py new file mode 100644 index 0000000000..37dee63f7f --- /dev/null +++ b/aws_lambda_powertools/utilities/feature_toggles/schema_fetcher.py @@ -0,0 +1,20 @@ +from abc import ABC, abstractclassmethod +from typing import Any, Dict + + +class SchemaFetcher(ABC): + def __init__(self, configuration_name: str, cache_seconds: int): + self.configuration_name = configuration_name + self._cache_seconds = cache_seconds + + @abstractclassmethod + def get_json_configuration(self) -> Dict[str, Any]: + """Get configuration string from any configuration storing service and return the parsed JSON dictionary + + Raises: + ConfigurationException: Any error that can occur during schema fetch or JSON parse + + Returns: + Dict[str, Any]: parsed JSON dictionary + """ + return None diff --git a/tests/functional/feature_toggles/test_feature_toggles.py b/tests/functional/feature_toggles/test_feature_toggles.py index 4c79c42abf..27f89eb151 100644 --- a/tests/functional/feature_toggles/test_feature_toggles.py +++ b/tests/functional/feature_toggles/test_feature_toggles.py @@ -3,6 +3,7 @@ import pytest # noqa: F401 from botocore.config import Config +from aws_lambda_powertools.utilities.feature_toggles.appconfig_fetcher import AppConfigFetcher from aws_lambda_powertools.utilities.feature_toggles.configuration_store import ConfigurationStore from aws_lambda_powertools.utilities.feature_toggles.schema import ACTION @@ -15,13 +16,15 @@ def config(): def init_configuration_store(mocker, mock_schema: Dict, config: Config) -> ConfigurationStore: mocked_get_conf = mocker.patch("aws_lambda_powertools.utilities.parameters.AppConfigProvider.get") mocked_get_conf.return_value = mock_schema - conf_store: ConfigurationStore = ConfigurationStore( + + app_conf_fetcher = AppConfigFetcher( environment="test_env", service="test_app", - conf_name="test_conf_name", + configuration_name="test_conf_name", cache_seconds=600, config=config, ) + conf_store: ConfigurationStore = ConfigurationStore(schema_fetcher=app_conf_fetcher) return conf_store diff --git a/tests/functional/feature_toggles/test_schema_validation.py b/tests/functional/feature_toggles/test_schema_validation.py index 9febae65ba..3b024c854b 100644 --- a/tests/functional/feature_toggles/test_schema_validation.py +++ b/tests/functional/feature_toggles/test_schema_validation.py @@ -5,12 +5,12 @@ from aws_lambda_powertools.utilities.feature_toggles.exceptions import ConfigurationException from aws_lambda_powertools.utilities.feature_toggles.schema import ( ACTION, - FEATURE_DEFAULT_VAL_KEY, - FEATURES_KEY, CONDITION_ACTION, CONDITION_KEY, CONDITION_VALUE, CONDITIONS_KEY, + FEATURE_DEFAULT_VAL_KEY, + FEATURES_KEY, RULE_DEFAULT_VALUE, RULE_NAME_KEY, RULES_KEY,