Skip to content

Commit 14af77f

Browse files
chore(feature_flags): warn on empty schema and empty rules (#8430)
* 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 * fix(feature_flags): emit user-facing schema warnings --------- Co-authored-by: Leandro <lcdama@amazon.pt>
1 parent 362a797 commit 14af77f

2 files changed

Lines changed: 109 additions & 7 deletions

File tree

aws_lambda_powertools/utilities/feature_flags/schema.py

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from __future__ import annotations
22

33
import logging
4+
import warnings
45
from datetime import datetime
56
from enum import Enum
67
from functools import lru_cache
@@ -10,6 +11,7 @@
1011

1112
from aws_lambda_powertools.utilities.feature_flags.base import BaseValidator
1213
from aws_lambda_powertools.utilities.feature_flags.exceptions import SchemaValidationError
14+
from aws_lambda_powertools.warnings import PowertoolsUserWarning
1315

1416
if TYPE_CHECKING:
1517
from aws_lambda_powertools.logging import Logger
@@ -212,6 +214,16 @@ def validate(self) -> None:
212214
if not isinstance(self.schema, dict):
213215
raise SchemaValidationError(f"Features must be a dictionary, schema={str(self.schema)}")
214216

217+
if not self.schema:
218+
# Often the result of an envelope query that matched nothing (e.g. a typo'd feature name).
219+
# Harmless for evaluation, so warn rather than raise.
220+
warnings.warn(
221+
"Feature flags schema is empty, no features to validate",
222+
category=PowertoolsUserWarning,
223+
stacklevel=2,
224+
)
225+
return
226+
215227
features = FeaturesValidator(schema=self.schema, logger=self.logger)
216228
features.validate()
217229

@@ -232,7 +244,12 @@ def validate(self):
232244
for name, feature in self.schema.items():
233245
self.logger.debug(f"Attempting to validate feature '{name}'")
234246
boolean_feature: bool = self.validate_feature(name, feature)
235-
rules = RulesValidator(feature=feature, boolean_feature=boolean_feature, logger=self.logger)
247+
rules = RulesValidator(
248+
feature=feature,
249+
boolean_feature=boolean_feature,
250+
logger=self.logger,
251+
feature_name=name,
252+
)
236253
rules.validate()
237254

238255
# returns True in case the feature is a regular feature flag with a boolean default value
@@ -260,16 +277,35 @@ def __init__(
260277
feature: dict[str, Any],
261278
boolean_feature: bool,
262279
logger: logging.Logger | Logger | None = None,
280+
feature_name: str | None = None,
263281
):
264282
self.feature = feature
265-
self.feature_name = next(iter(self.feature))
283+
self.feature_name = feature_name if feature_name is not None else next(iter(self.feature))
266284
self.rules: dict | None = self.feature.get(RULES_KEY)
267285
self.logger = logger or LOGGER
268286
self.boolean_feature = boolean_feature
269287

270288
def validate(self):
271289
if not self.rules:
272-
self.logger.debug("Rules are empty, ignoring validation")
290+
if RULES_KEY in self.feature:
291+
# 'rules' was authored but is empty (e.g. {}, [], None). Evaluation falls back to 'default',
292+
# so this is harmless, but it likely signals a mistake. A non-dict type is called out separately
293+
# because a non-empty value of that type would be rejected below.
294+
if isinstance(self.rules, dict) or self.rules is None:
295+
warnings.warn(
296+
f"Feature has 'rules' but it is empty, feature={self.feature_name}",
297+
category=PowertoolsUserWarning,
298+
stacklevel=2,
299+
)
300+
else:
301+
warnings.warn(
302+
f"Feature 'rules' should be a dictionary but is an empty {type(self.rules).__name__}, "
303+
f"feature={self.feature_name}",
304+
category=PowertoolsUserWarning,
305+
stacklevel=2,
306+
)
307+
else:
308+
self.logger.debug("Rules are empty, ignoring validation")
273309
return
274310

275311
if not isinstance(self.rules, dict):

tests/functional/feature_flags/_boto3/test_schema_validation.py

Lines changed: 70 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from __future__ import annotations
22

33
import re
4+
import warnings
45

56
import pytest
67

@@ -24,6 +25,7 @@
2425
TimeKeys,
2526
TimeValues,
2627
)
28+
from aws_lambda_powertools.warnings import PowertoolsUserWarning
2729

2830
EMPTY_SCHEMA = {"": ""}
2931

@@ -34,9 +36,33 @@ def test_invalid_features_dict():
3436
validator.validate()
3537

3638

37-
def test_empty_features_not_fail():
39+
def test_empty_features_emits_warning():
3840
validator = SchemaValidator(schema={})
39-
validator.validate()
41+
42+
with pytest.warns(PowertoolsUserWarning, match="Feature flags schema is empty"):
43+
validator.validate()
44+
45+
46+
def test_features_not_empty_no_warning():
47+
# GIVEN a well-formed document with rules
48+
schema = {
49+
"my_feature": {
50+
FEATURE_DEFAULT_VAL_KEY: False,
51+
RULES_KEY: {
52+
"tenant match": {
53+
RULE_MATCH_VALUE: True,
54+
CONDITIONS_KEY: [
55+
{CONDITION_ACTION: RuleAction.EQUALS.value, CONDITION_KEY: "tenant_id", CONDITION_VALUE: "6"},
56+
],
57+
},
58+
},
59+
},
60+
}
61+
62+
# WHEN validating, THEN no warning is emitted
63+
with warnings.catch_warnings():
64+
warnings.simplefilter("error", PowertoolsUserWarning)
65+
SchemaValidator(schema).validate()
4066

4167

4268
@pytest.mark.parametrize(
@@ -59,14 +85,51 @@ def test_valid_feature_dict():
5985
# empty rules list
6086
schema = {"my_feature": {FEATURE_DEFAULT_VAL_KEY: False, RULES_KEY: []}}
6187
validator = SchemaValidator(schema)
62-
validator.validate()
88+
with pytest.warns(PowertoolsUserWarning, match="empty list"):
89+
validator.validate()
6390

6491
# no rules list at all
6592
schema = {"my_feature": {FEATURE_DEFAULT_VAL_KEY: False}}
6693
validator = SchemaValidator(schema)
6794
validator.validate()
6895

6996

97+
@pytest.mark.parametrize(
98+
"rules, expected_message",
99+
[
100+
pytest.param({}, "Feature has 'rules' but it is empty, feature=my_feature", id="empty_dict"),
101+
pytest.param(None, "Feature has 'rules' but it is empty, feature=my_feature", id="none"),
102+
pytest.param(
103+
[],
104+
"Feature 'rules' should be a dictionary but is an empty list, feature=my_feature",
105+
id="empty_list",
106+
),
107+
pytest.param(
108+
"",
109+
"Feature 'rules' should be a dictionary but is an empty str, feature=my_feature",
110+
id="empty_str",
111+
),
112+
],
113+
)
114+
def test_feature_with_empty_rules_emits_warning(rules, expected_message):
115+
# GIVEN a feature whose 'rules' key is present but falsy
116+
schema = {"my_feature": {FEATURE_DEFAULT_VAL_KEY: False, RULES_KEY: rules}}
117+
118+
# WHEN validating, THEN a warning naming the feature is emitted and nothing is raised
119+
with pytest.warns(PowertoolsUserWarning, match=re.escape(expected_message)):
120+
SchemaValidator(schema).validate()
121+
122+
123+
def test_feature_without_rules_key_no_warning():
124+
# GIVEN a feature that simply omits 'rules'
125+
schema = {"my_feature": {FEATURE_DEFAULT_VAL_KEY: False}}
126+
127+
# WHEN validating, THEN no warning is emitted; omitting rules is the documented way to declare a static flag
128+
with warnings.catch_warnings():
129+
warnings.simplefilter("error", PowertoolsUserWarning)
130+
SchemaValidator(schema).validate()
131+
132+
70133
def test_invalid_feature_default_value_is_not_boolean():
71134
# feature is boolean but default value is a number, not a boolean
72135
schema = {"my_feature": {FEATURE_DEFAULT_VAL_KEY: 3, FEATURE_DEFAULT_VAL_TYPE_KEY: True, RULES_KEY: []}}
@@ -87,7 +150,10 @@ def test_invalid_rule():
87150
},
88151
}
89152
validator = SchemaValidator(schema)
90-
with pytest.raises(SchemaValidationError):
153+
with pytest.raises(
154+
SchemaValidationError,
155+
match="Feature rules must be a dictionary, feature=my_feature",
156+
):
91157
validator.validate()
92158

93159
# rules RULE_MATCH_VALUE is not bool

0 commit comments

Comments
 (0)