diff --git a/aws_lambda_powertools/utilities/feature_flags/schema.py b/aws_lambda_powertools/utilities/feature_flags/schema.py index a8739d5eb05..b6ca68013ce 100644 --- a/aws_lambda_powertools/utilities/feature_flags/schema.py +++ b/aws_lambda_powertools/utilities/feature_flags/schema.py @@ -212,6 +212,12 @@ def validate(self) -> None: if not isinstance(self.schema, dict): raise SchemaValidationError(f"Features must be a dictionary, schema={str(self.schema)}") + if not self.schema: + # Often the result of an envelope query that matched nothing (e.g. a typo'd feature name). + # Harmless for evaluation, so warn rather than raise. + self.logger.warning("Feature flags schema is empty, no features to validate") + return + features = FeaturesValidator(schema=self.schema, logger=self.logger) features.validate() @@ -232,7 +238,12 @@ def validate(self): for name, feature in self.schema.items(): self.logger.debug(f"Attempting to validate feature '{name}'") boolean_feature: bool = self.validate_feature(name, feature) - rules = RulesValidator(feature=feature, boolean_feature=boolean_feature, logger=self.logger) + rules = RulesValidator( + feature=feature, + boolean_feature=boolean_feature, + logger=self.logger, + feature_name=name, + ) rules.validate() # returns True in case the feature is a regular feature flag with a boolean default value @@ -260,16 +271,29 @@ def __init__( feature: dict[str, Any], boolean_feature: bool, logger: logging.Logger | Logger | None = None, + feature_name: str | None = None, ): self.feature = feature - self.feature_name = next(iter(self.feature)) + self.feature_name = feature_name if feature_name is not None else next(iter(self.feature)) self.rules: dict | None = self.feature.get(RULES_KEY) self.logger = logger or LOGGER self.boolean_feature = boolean_feature def validate(self): if not self.rules: - self.logger.debug("Rules are empty, ignoring validation") + if RULES_KEY in self.feature: + # 'rules' was authored but is empty (e.g. {}, [], None). Evaluation falls back to 'default', + # so this is harmless, but it likely signals a mistake. A non-dict type is called out separately + # because a non-empty value of that type would be rejected below. + if isinstance(self.rules, dict) or self.rules is None: + self.logger.warning(f"Feature has 'rules' but it is empty, feature={self.feature_name}") + else: + self.logger.warning( + f"Feature 'rules' should be a dictionary but is an empty {type(self.rules).__name__}, " + f"feature={self.feature_name}", + ) + else: + self.logger.debug("Rules are empty, ignoring validation") return if not isinstance(self.rules, dict): diff --git a/tests/functional/feature_flags/_boto3/test_schema_validation.py b/tests/functional/feature_flags/_boto3/test_schema_validation.py index afc7130505e..21166c55230 100644 --- a/tests/functional/feature_flags/_boto3/test_schema_validation.py +++ b/tests/functional/feature_flags/_boto3/test_schema_validation.py @@ -1,5 +1,6 @@ from __future__ import annotations +import logging import re import pytest @@ -39,6 +40,44 @@ def test_empty_features_not_fail(): validator.validate() +def test_empty_features_emits_warning(caplog): + # GIVEN an empty top-level document, e.g. the result of an envelope query that matched nothing + validator = SchemaValidator(schema={}) + + # WHEN validating + with caplog.at_level(logging.WARNING): + validator.validate() + + # THEN a warning is emitted and nothing is raised + assert len(caplog.records) == 1 + assert caplog.records[0].levelno == logging.WARNING + assert "schema is empty" in caplog.records[0].getMessage() + + +def test_features_not_empty_no_warning(caplog): + # GIVEN a well-formed document with rules + schema = { + "my_feature": { + FEATURE_DEFAULT_VAL_KEY: False, + RULES_KEY: { + "tenant match": { + RULE_MATCH_VALUE: True, + CONDITIONS_KEY: [ + {CONDITION_ACTION: RuleAction.EQUALS.value, CONDITION_KEY: "tenant_id", CONDITION_VALUE: "6"}, + ], + }, + }, + }, + } + + # WHEN validating + with caplog.at_level(logging.WARNING): + SchemaValidator(schema).validate() + + # THEN no warning is emitted + assert not caplog.records + + @pytest.mark.parametrize( "schema", [ @@ -67,6 +106,49 @@ def test_valid_feature_dict(): validator.validate() +@pytest.mark.parametrize( + "rules, expected_message", + [ + pytest.param({}, "Feature has 'rules' but it is empty, feature=my_feature", id="empty_dict"), + pytest.param(None, "Feature has 'rules' but it is empty, feature=my_feature", id="none"), + pytest.param( + [], + "Feature 'rules' should be a dictionary but is an empty list, feature=my_feature", + id="empty_list", + ), + pytest.param( + "", + "Feature 'rules' should be a dictionary but is an empty str, feature=my_feature", + id="empty_str", + ), + ], +) +def test_feature_with_empty_rules_emits_warning(caplog, rules, expected_message): + # GIVEN a feature whose 'rules' key is present but falsy + schema = {"my_feature": {FEATURE_DEFAULT_VAL_KEY: False, RULES_KEY: rules}} + + # WHEN validating + with caplog.at_level(logging.WARNING): + SchemaValidator(schema).validate() + + # THEN a warning naming the feature is emitted and nothing is raised + assert len(caplog.records) == 1 + assert caplog.records[0].levelno == logging.WARNING + assert caplog.records[0].getMessage() == expected_message + + +def test_feature_without_rules_key_no_warning(caplog): + # GIVEN a feature that simply omits 'rules' + schema = {"my_feature": {FEATURE_DEFAULT_VAL_KEY: False}} + + # WHEN validating + with caplog.at_level(logging.WARNING): + SchemaValidator(schema).validate() + + # THEN no warning is emitted; omitting rules is the documented way to declare a static flag + assert not caplog.records + + def test_invalid_feature_default_value_is_not_boolean(): # feature is boolean but default value is a number, not a boolean schema = {"my_feature": {FEATURE_DEFAULT_VAL_KEY: 3, FEATURE_DEFAULT_VAL_TYPE_KEY: True, RULES_KEY: []}}