diff --git a/.gitmodules b/.gitmodules index 8fda0192..08ad70f5 100644 --- a/.gitmodules +++ b/.gitmodules @@ -8,4 +8,4 @@ [submodule "providers/openfeature-provider-flagd/openfeature/test-harness"] path = providers/openfeature-provider-flagd/openfeature/test-harness url = https://github.com/open-feature/flagd-testbed.git - branch = v2.8.0 + branch = v2.10.2 diff --git a/providers/openfeature-provider-flagd/openfeature/test-harness b/providers/openfeature-provider-flagd/openfeature/test-harness index 8dca72ca..59c3c3cc 160000 --- a/providers/openfeature-provider-flagd/openfeature/test-harness +++ b/providers/openfeature-provider-flagd/openfeature/test-harness @@ -1 +1 @@ -Subproject commit 8dca72ca3877e009b3b76664749f3f604403dfd6 +Subproject commit 59c3c3ccfb018db82281684d231067e332c8103d diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py index 4b70afdd..af9838c5 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py @@ -5,13 +5,13 @@ ) from openfeature.evaluation_context import EvaluationContext from openfeature.event import ProviderEventDetails -from openfeature.exception import FlagNotFoundError, ParseError +from openfeature.exception import ErrorCode, FlagNotFoundError, GeneralError, ParseError from openfeature.flag_evaluation import FlagResolutionDetails, Reason from ..config import Config from .process.connector import FlagStateConnector from .process.connector.grpc_watcher import GrpcWatcher -from .process.flags import FlagStore +from .process.flags import Flag, FlagStore from .process.targeting import targeting T = typing.TypeVar("T") @@ -128,26 +128,30 @@ def _resolve( ) if not flag.targeting: - variant, value = flag.default - return FlagResolutionDetails( - value, variant=variant, flag_metadata=metadata, reason=Reason.STATIC - ) + return _default_resolve(flag, metadata, Reason.STATIC) - variant = targeting(flag.key, flag.targeting, evaluation_context) + try: + variant = targeting(flag.key, flag.targeting, evaluation_context) + if variant is None: + return _default_resolve(flag, metadata, Reason.DEFAULT) - if variant is None: - variant, value = flag.default - return FlagResolutionDetails( - value, variant=variant, flag_metadata=metadata, reason=Reason.DEFAULT - ) - if not isinstance(variant, (str, bool)): - raise ParseError( - "Parsed JSONLogic targeting did not return a string or bool" - ) + # convert to string to support shorthand (boolean in python is with capital T hence the special case) + if isinstance(variant, bool): + variant = str(variant).lower() + elif not isinstance(variant, str): + variant = str(variant) + + if variant not in flag.variants: + raise GeneralError( + f"Resolved variant {variant} not in variants config." + ) + + except ReferenceError: + raise ParseError(f"Invalid targeting {targeting}") from ReferenceError variant, value = flag.get_variant(variant) if value is None: - raise ParseError(f"Resolved variant {variant} not in variants config.") + raise GeneralError(f"Resolved variant {variant} not in variants config.") return FlagResolutionDetails( value, @@ -155,3 +159,24 @@ def _resolve( reason=Reason.TARGETING_MATCH, flag_metadata=metadata, ) + + +def _default_resolve( + flag: Flag, + metadata: typing.Mapping[str, typing.Union[float, int, str, bool]], + reason: Reason, +) -> FlagResolutionDetails: + variant, value = flag.default + if variant is None: + return FlagResolutionDetails( + value, + variant=variant, + reason=Reason.ERROR, + error_code=ErrorCode.FLAG_NOT_FOUND, + flag_metadata=metadata, + ) + if variant not in flag.variants: + raise GeneralError(f"Resolved variant {variant} not in variants config.") + return FlagResolutionDetails( + value, variant=variant, flag_metadata=metadata, reason=reason + ) diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/flags.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/flags.py index 9559761e..3f448c7d 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/flags.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/flags.py @@ -72,30 +72,22 @@ class Flag: key: str state: str variants: typing.Mapping[str, typing.Any] - default_variant: typing.Union[bool, str] + default_variant: typing.Optional[typing.Union[bool, str]] = None targeting: typing.Optional[dict] = None metadata: typing.Optional[ typing.Mapping[str, typing.Union[float, int, str, bool]] ] = None def __post_init__(self) -> None: - if not self.state or not isinstance(self.state, str): + if not self.state or not (self.state == "ENABLED" or self.state == "DISABLED"): raise ParseError("Incorrect 'state' value provided in flag config") if not self.variants or not isinstance(self.variants, dict): raise ParseError("Incorrect 'variants' value provided in flag config") - if not self.default_variant or not isinstance( - self.default_variant, (str, bool) - ): + if self.default_variant and not isinstance(self.default_variant, (str, bool)): raise ParseError("Incorrect 'defaultVariant' value provided in flag config") - if self.targeting and not isinstance(self.targeting, dict): - raise ParseError("Incorrect 'targeting' value provided in flag config") - - if self.default_variant not in self.variants: - raise ParseError("Default variant does not match set of variants") - if self.metadata: if not isinstance(self.metadata, dict): raise ParseError("Flag metadata is not a valid json object") @@ -106,6 +98,8 @@ def __post_init__(self) -> None: def from_dict(cls, key: str, data: dict) -> "Flag": if "defaultVariant" in data: data["default_variant"] = data["defaultVariant"] + if data["default_variant"] == "": + data["default_variant"] = None del data["defaultVariant"] data.pop("source", None) @@ -119,13 +113,16 @@ def from_dict(cls, key: str, data: dict) -> "Flag": raise ParseError from err @property - def default(self) -> tuple[str, typing.Any]: + def default(self) -> tuple[typing.Optional[str], typing.Any]: return self.get_variant(self.default_variant) def get_variant( - self, variant_key: typing.Union[str, bool] - ) -> tuple[str, typing.Any]: + self, variant_key: typing.Union[str, bool, None] + ) -> tuple[typing.Optional[str], typing.Any]: if isinstance(variant_key, bool): variant_key = str(variant_key).lower() + if not variant_key: + return None, None + return variant_key, self.variants.get(variant_key) diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/targeting.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/targeting.py index bcc00a5a..fb55cab9 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/targeting.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/targeting.py @@ -5,6 +5,7 @@ from json_logic.types import JsonValue from openfeature.evaluation_context import EvaluationContext +from openfeature.exception import ParseError from .custom_ops import ( ends_with, @@ -27,6 +28,9 @@ def targeting( targeting: dict, evaluation_context: typing.Optional[EvaluationContext] = None, ) -> JsonValue: + if not isinstance(targeting, dict): + raise ParseError(f"Invalid 'targeting' value in flag: {targeting}") + json_logic_context = evaluation_context.attributes if evaluation_context else {} json_logic_context["$flagd"] = {"flagKey": key, "timestamp": int(time.time())} json_logic_context["targetingKey"] = ( diff --git a/providers/openfeature-provider-flagd/tests/e2e/flagd_container.py b/providers/openfeature-provider-flagd/tests/e2e/flagd_container.py index 2a01d503..e0364102 100644 --- a/providers/openfeature-provider-flagd/tests/e2e/flagd_container.py +++ b/providers/openfeature-provider-flagd/tests/e2e/flagd_container.py @@ -48,7 +48,7 @@ def start(self) -> "FlagdContainer": return self @wait_container_is_ready(ConnectionError) - def _checker(self, host: str, port: str) -> None: + def _checker(self, host: str, port: int) -> None: # First we wait for Flagd to say it's listening wait_for_logs( self, @@ -58,7 +58,7 @@ def _checker(self, host: str, port: str) -> None: time.sleep(1) # Second we use the GRPC health check endpoint - with grpc.insecure_channel(host + ":" + port) as channel: + with grpc.insecure_channel(host + ":" + str(port)) as channel: health_stub = health_pb2_grpc.HealthStub(channel) def health_check_call(stub: health_pb2_grpc.HealthStub): diff --git a/providers/openfeature-provider-flagd/tests/e2e/step/context_steps.py b/providers/openfeature-provider-flagd/tests/e2e/step/context_steps.py index 65f34d60..62730a00 100644 --- a/providers/openfeature-provider-flagd/tests/e2e/step/context_steps.py +++ b/providers/openfeature-provider-flagd/tests/e2e/step/context_steps.py @@ -36,6 +36,18 @@ def update_context( evaluation_context.attributes[key] = type_cast[type_info](value) +@given( + parsers.cfparse( + 'a context containing a key "{key}", with type "{type_info}" and with value ""' + ), +) +def update_context_without_value( + evaluation_context: EvaluationContext, key: str, type_info: str +): + """a context containing a key and value.""" + update_context(evaluation_context, key, type_info, "") + + @when( parsers.cfparse( 'context contains keys {fields:s} with values "{svalue}", "{svalue2}", {ivalue:d}, "{bvalue:bool}"', diff --git a/providers/openfeature-provider-flagd/tests/e2e/step/flag_step.py b/providers/openfeature-provider-flagd/tests/e2e/step/flag_step.py index e0ff97ce..1827055d 100644 --- a/providers/openfeature-provider-flagd/tests/e2e/step/flag_step.py +++ b/providers/openfeature-provider-flagd/tests/e2e/step/flag_step.py @@ -1,5 +1,5 @@ import requests -from asserts import assert_equal +from asserts import assert_equal, assert_is_none from pytest_bdd import given, parsers, then, when from openfeature.client import OpenFeatureClient @@ -96,6 +96,25 @@ def resolve_details_reason( assert_equal(details.reason, Reason(reason)) +@then( + parsers.cfparse('the error-code should be "{error_code}"'), +) +def resolve_details_error_code( + details: FlagEvaluationDetails[JsonPrimitive], + error_code: str, +): + assert_equal(details.error_code, error_code) + + +@then( + parsers.cfparse('the error-code should be ""'), +) +def resolve_details_empty_error_code( + details: FlagEvaluationDetails[JsonPrimitive], +): + assert_is_none(details.error_code) + + @then(parsers.cfparse("the resolved metadata should contain")) def metadata_contains(details: FlagEvaluationDetails[JsonPrimitive], datatable): assert_equal(len(details.flag_metadata), len(datatable) - 1) # skip table header diff --git a/providers/openfeature-provider-flagd/tests/flags/basic-flag-without-default.json b/providers/openfeature-provider-flagd/tests/flags/basic-flag-without-default.json new file mode 100644 index 00000000..359cc4e5 --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/flags/basic-flag-without-default.json @@ -0,0 +1,12 @@ +{ + "flags": { + "basic-flag": { + "state": "ENABLED", + "variants": { + "true": true, + "false": false + }, + "targeting": {} + } + } +} \ No newline at end of file diff --git a/providers/openfeature-provider-flagd/tests/test_errors.py b/providers/openfeature-provider-flagd/tests/test_errors.py index 872994c8..dc5fded2 100644 --- a/providers/openfeature-provider-flagd/tests/test_errors.py +++ b/providers/openfeature-provider-flagd/tests/test_errors.py @@ -23,11 +23,9 @@ def create_client(provider: FlagdProvider): "not-a-flag.json", "basic-flag-wrong-structure.json", "basic-flag-invalid.not-json", - "basic-flag-wrong-variant.json", "basic-flag-broken-state.json", "basic-flag-broken-variants.json", "basic-flag-broken-default.json", - "basic-flag-broken-targeting.json", ], ) def test_file_load_errors(file_name: str): @@ -46,6 +44,38 @@ def test_file_load_errors(file_name: str): assert res.error_code == ErrorCode.FLAG_NOT_FOUND +def test_non_existent_variant(): + path = os.path.abspath(os.path.join(os.path.dirname(__file__), "./flags/")) + client = create_client( + FlagdProvider( + resolver_type=ResolverType.IN_PROCESS, + offline_flag_source_path=f"{path}/basic-flag-wrong-variant.json", + ) + ) + + res = client.get_boolean_details("basic-flag", False) + + assert res.value is False + assert res.reason == Reason.ERROR + assert res.error_code == ErrorCode.GENERAL + + +def test_broken_targeting(): + path = os.path.abspath(os.path.join(os.path.dirname(__file__), "./flags/")) + client = create_client( + FlagdProvider( + resolver_type=ResolverType.IN_PROCESS, + offline_flag_source_path=f"{path}/basic-flag-broken-targeting.json", + ) + ) + + res = client.get_boolean_details("basic-flag", False) + + assert res.value is False + assert res.reason == Reason.ERROR + assert res.error_code == ErrorCode.PARSE_ERROR + + @pytest.mark.parametrize( "file_name", [ diff --git a/providers/openfeature-provider-flagd/tests/test_file_store.py b/providers/openfeature-provider-flagd/tests/test_file_store.py index 7af4ccfd..bdc8cf62 100644 --- a/providers/openfeature-provider-flagd/tests/test_file_store.py +++ b/providers/openfeature-provider-flagd/tests/test_file_store.py @@ -21,6 +21,7 @@ def create_client(provider: FlagdProvider): "file_name", [ "basic-flag.json", + "basic-flag-without-default.json", "basic-flag.yaml", ], ) diff --git a/providers/openfeature-provider-flagd/tests/test_in_process.py b/providers/openfeature-provider-flagd/tests/test_in_process.py index cc9a67fd..cec882f8 100644 --- a/providers/openfeature-provider-flagd/tests/test_in_process.py +++ b/providers/openfeature-provider-flagd/tests/test_in_process.py @@ -6,7 +6,7 @@ from openfeature.contrib.provider.flagd.resolvers.in_process import InProcessResolver from openfeature.contrib.provider.flagd.resolvers.process.flags import Flag, FlagStore from openfeature.evaluation_context import EvaluationContext -from openfeature.exception import FlagNotFoundError, ParseError +from openfeature.exception import FlagNotFoundError, GeneralError def targeting(): @@ -79,7 +79,7 @@ def test_resolve_boolean_details_invalid_variant(resolver, flag): resolver.flag_store.get_flag = Mock(return_value=flag) - with pytest.raises(ParseError): + with pytest.raises(GeneralError): resolver.resolve_boolean_details("flag", False)