From c4cd833a8bbe73b42dd6bf128e13b8f06640a0ce Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Thu, 3 Sep 2026 18:24:52 +0200 Subject: [PATCH] chore(feature_flags): warn on empty schema and empty rules SchemaValidator accepted an empty top-level document and any feature whose 'rules' key was present but falsy (including a list) without any signal. These are harmless for evaluation, but they usually indicate an authoring mistake such as a typo'd envelope path. Emit a warning log for an empty schema, for a feature whose 'rules' is present but empty, and a more specific warning when the empty value is not a dictionary. Nothing is raised, so existing documents keep validating. Omitting 'rules' entirely stays silent since that is the documented way to declare a static flag. Pass the real feature name into RulesValidator. It previously derived the name from the feature's first key (usually 'default'), so error and warning messages named the wrong thing. Closes #8427 --- .../utilities/feature_flags/schema.py | 30 ++++++- .../_boto3/test_schema_validation.py | 82 +++++++++++++++++++ 2 files changed, 109 insertions(+), 3 deletions(-) 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: []}}