Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 39 additions & 3 deletions aws_lambda_powertools/utilities/feature_flags/schema.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import logging
import warnings
from datetime import datetime
from enum import Enum
from functools import lru_cache
Expand All @@ -10,6 +11,7 @@

from aws_lambda_powertools.utilities.feature_flags.base import BaseValidator
from aws_lambda_powertools.utilities.feature_flags.exceptions import SchemaValidationError
from aws_lambda_powertools.warnings import PowertoolsUserWarning

if TYPE_CHECKING:
from aws_lambda_powertools.logging import Logger
Expand Down Expand Up @@ -212,6 +214,16 @@ 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.
warnings.warn(
"Feature flags schema is empty, no features to validate",
category=PowertoolsUserWarning,
stacklevel=2,
)
return

features = FeaturesValidator(schema=self.schema, logger=self.logger)
features.validate()

Expand All @@ -232,7 +244,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
Expand Down Expand Up @@ -260,16 +277,35 @@ 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:
warnings.warn(
f"Feature has 'rules' but it is empty, feature={self.feature_name}",
category=PowertoolsUserWarning,
stacklevel=2,
)
else:
warnings.warn(
f"Feature 'rules' should be a dictionary but is an empty {type(self.rules).__name__}, "
f"feature={self.feature_name}",
category=PowertoolsUserWarning,
stacklevel=2,
)
else:
self.logger.debug("Rules are empty, ignoring validation")
return

if not isinstance(self.rules, dict):
Expand Down
74 changes: 70 additions & 4 deletions tests/functional/feature_flags/_boto3/test_schema_validation.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import re
import warnings

import pytest

Expand All @@ -24,6 +25,7 @@
TimeKeys,
TimeValues,
)
from aws_lambda_powertools.warnings import PowertoolsUserWarning

EMPTY_SCHEMA = {"": ""}

Expand All @@ -34,9 +36,33 @@
validator.validate()


def test_empty_features_not_fail():
def test_empty_features_emits_warning():
validator = SchemaValidator(schema={})
validator.validate()

with pytest.warns(PowertoolsUserWarning, match="Feature flags schema is empty"):
validator.validate()


def test_features_not_empty_no_warning():
# 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, THEN no warning is emitted
with warnings.catch_warnings():
warnings.simplefilter("error", PowertoolsUserWarning)
SchemaValidator(schema).validate()


@pytest.mark.parametrize(
Expand All @@ -59,14 +85,51 @@
# empty rules list
schema = {"my_feature": {FEATURE_DEFAULT_VAL_KEY: False, RULES_KEY: []}}
validator = SchemaValidator(schema)
validator.validate()
with pytest.warns(PowertoolsUserWarning, match="empty list"):
validator.validate()

# no rules list at all
schema = {"my_feature": {FEATURE_DEFAULT_VAL_KEY: False}}
validator = SchemaValidator(schema)
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(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, THEN a warning naming the feature is emitted and nothing is raised
with pytest.warns(PowertoolsUserWarning, match=re.escape(expected_message)):

Check warning on line 119 in tests/functional/feature_flags/_boto3/test_schema_validation.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this warning test to have only one invocation possibly emitting a warning.

See more on https://sonarcloud.io/project/issues?id=aws-powertools_powertools-lambda-python&issues=AaCg2ntgdBIFDwPvQXQ4&open=AaCg2ntgdBIFDwPvQXQ4&pullRequest=8430
SchemaValidator(schema).validate()


def test_feature_without_rules_key_no_warning():
# GIVEN a feature that simply omits 'rules'
schema = {"my_feature": {FEATURE_DEFAULT_VAL_KEY: False}}

# WHEN validating, THEN no warning is emitted; omitting rules is the documented way to declare a static flag
with warnings.catch_warnings():
warnings.simplefilter("error", PowertoolsUserWarning)
SchemaValidator(schema).validate()


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: []}}
Expand All @@ -87,7 +150,10 @@
},
}
validator = SchemaValidator(schema)
with pytest.raises(SchemaValidationError):
with pytest.raises(
SchemaValidationError,
match="Feature rules must be a dictionary, feature=my_feature",
):
validator.validate()

# rules RULE_MATCH_VALUE is not bool
Expand Down