diff --git a/providers/openfeature-provider-flagd/README.md b/providers/openfeature-provider-flagd/README.md index fbade51f..3d382009 100644 --- a/providers/openfeature-provider-flagd/README.md +++ b/providers/openfeature-provider-flagd/README.md @@ -182,6 +182,46 @@ api.set_provider(FlagdProvider( )) ``` +## Provider conformance suite + +This provider runs the [OpenFeature Provider Conformance Suite][tck] against a flagd-testbed stack, +once per resolver, in `tests/tck`. The suite owns the container stack: `tests/tck/conftest.py` +declares a Compose file and the two ports the resolvers connect to, and nothing else. + +`tests/tck/docker-compose.yaml` is one definition of the backend for the whole repository, and the +OFREP adoption carries a byte-identical copy — each provider package publishes its own distribution +and must not read the other's files, so the two are kept in step by `diff` rather than by sharing a +path. Change one, copy it to the other. + +**It is excluded from the default build, and a maintainer runs it by hand before merging a change to +it.** + +``` +poe test-tck # both resolvers, needs Docker +poe test # everything else, which is what CI runs +``` + +The exclusion lives in `pyproject.toml`: `--ignore=tests/tck` on the two tasks `build.yml` reaches, +with the reason in a comment above them. Why a conformance suite is not a required gate is +[Appendix F, "Running the suite in CI"][appendix-f], and is not restated here. + +Two things that are this provider's rather than the policy's: + +- **Docker is not what decides it.** `tests/e2e` needs Docker too and does run in the default build. + What decides it is the run: **8 failed, 119 passed, 3 skipped**, being three canonical flags that + no released flagd-testbed seeds yet, failing on each resolver, plus two genuine provider-side + gaps — one per resolver, and only one of them carries a known-deviation entry. + `tests/tck/conftest.py` accounts for all eight individually, with the flag or requirement each one + turns on, so a reviewer running the suite can tell a new failure from a known one and the number + above is what to expect. +- **The default build still collects the suite** — `poe test` and `poe test-cov` end in + `pytest tests/tck --collect-only`, which imports every module and starts no container. An excluded + suite that has quietly stopped importing against the harness is worse than one that runs and + fails, and `mypy` here is configured over `src` alone, so nothing else would notice. + +[tck]: ../../tools/openfeature-tck/README.md +[appendix-f]: https://github.com/open-feature/spec/blob/main/specification/appendix-f-provider-conformance.md + ## License Apache 2.0 - See [LICENSE](./LICENSE) for more information. diff --git a/providers/openfeature-provider-flagd/pyproject.toml b/providers/openfeature-provider-flagd/pyproject.toml index 2c1c1b3a..46ba6681 100644 --- a/providers/openfeature-provider-flagd/pyproject.toml +++ b/providers/openfeature-provider-flagd/pyproject.toml @@ -35,6 +35,12 @@ dev = [ "coverage[toml]>=7.10.0,<8.0.0", "grpcio-health-checking>=1.82.1,<2.0.0", "mypy>=1.18.0,<2.0.0", + # The OpenFeature conformance suite. Ships the feature files, the flag set, + # the control-API client and the Compose harness that owns the container + # stack, and registers its step definitions through a pytest11 entry point -- + # so tests/tck declares a Compose file and nothing else. The `compose` extra + # is what pulls testcontainers in for the harness. + "openfeature-tck[compose]", "poethepoet>=0.37.0", "pytest>=9.0.0,<10.0.0", "pytest-bdd>=8.1.0,<9.0.0", @@ -112,13 +118,58 @@ warn_unused_ignores = false [tool.uv.sources] openfeature-flagd-core = { workspace = true } +openfeature-tck = { workspace = true } [tool.pytest] strict = true [tool.poe.tasks] -test = "pytest tests" -test-cov = "coverage run -m pytest tests" +# `tests/tck` is excluded from the default build on purpose, and a maintainer +# runs `poe test-tck` by hand before merging a change to it. Why a conformance +# suite is not a required gate is Appendix F, "Running the suite in CI" -- +# linked from tools/openfeature-tck/README.md -- and is deliberately not +# restated here. +# +# What is local to this package: a full run is 8 failed, 119 passed, 3 skipped. +# Six of those failures are canonical flags flagd-testbed does not seed yet, on +# each of the two resolvers; the other two are real provider-side gaps, one per +# resolver. tests/tck/conftest.py accounts for all eight and for the three +# skips. Docker is not what decides the exclusion -- `tests/e2e` needs Docker +# too and does run here. +# +# The suite is still *collected* on every default build, so it cannot quietly +# stop importing against the harness while nobody runs it. `--ignore` would +# otherwise leave nothing checking that, since mypy here is configured over +# `src` alone. +# +# `ignore_fail = "return_non_zero"` is what makes that true rather than +# aspirational: poe aborts a sequence at its first failing subtask, and this +# package's default suite is red on this branch -- the TCK requires +# openfeature-sdk >= 0.10 and this provider is not updated for it yet -- so the +# collect step sat behind a step that never succeeds and never ran. Every +# subtask now runs and a non-zero exit still propagates. +test = { sequence = ["test-default", "test-tck-collect"], ignore_fail = "return_non_zero" } +test-cov = { sequence = ["test-cov-default", "test-tck-collect"], ignore_fail = "return_non_zero" } +test-default = "pytest tests --ignore=tests/tck" +test-cov-default = "coverage run -m pytest tests --ignore=tests/tck" +# The sync is part of the run rather than something a maintainer remembers, +# because Appendix F requires the revision check to be in force **where the +# scenarios execute** and not only in the TCK implementation's own tests. This +# is a uv workspace, so `openfeature-tck` is installed editable and the +# canonical assets this suite collects are the gitignored copies under +# `tools/openfeature-tck/src`: a rebase that moves the submodule pin leaves them +# from the previous revision, and a suite run from here would neither notice nor +# say so. That is exactly how an adoption in another language published a full +# pass against stale scenarios. `hatch_build_sync.py` checks the submodule out +# at the pin, refuses to copy if it cannot, and needs nothing but the standard +# library. +# +# Not on `test-tck-collect`, deliberately: that runs on every default build, it +# executes no scenario, and it publishes no result, so the rule does not reach +# it and the default build stays free of a submodule operation. +sync-spec-assets = "python ../../tools/openfeature-tck/hatch_build_sync.py" +test-tck = ["sync-spec-assets", {cmd = "pytest tests/tck"}] +test-tck-collect = "pytest tests/tck --collect-only -q" cov-report = "coverage xml" cov = [ "test-cov", diff --git a/providers/openfeature-provider-flagd/tests/tck/__init__.py b/providers/openfeature-provider-flagd/tests/tck/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/providers/openfeature-provider-flagd/tests/tck/conftest.py b/providers/openfeature-provider-flagd/tests/tck/conftest.py new file mode 100644 index 00000000..b577d4c3 --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/tck/conftest.py @@ -0,0 +1,106 @@ +"""One testbed stack, declared rather than wired, and shared by both suites. + +The container lifecycle belongs to the TCK -- see its README for what +``tck_backend`` does with the declaration below, and Appendix F, "The control +API", for why the stack is started once and never restarted. What is left here +is the declaration and one free port for the scenarios that need a backend that +is not there. + +One stack and one ``HttpControl`` serve both suites, because one flagd process +serves both resolver ports: 8013 for RPC and 8015 for sync. The launchpad +registers no ``/reset`` and no ``/healthz``, so every ``prepare_scenario`` takes +the harness's documented ``/start`` fallback and one 404 is logged per session. + +**What a full run reports, and what each failure is.** ``8 failed, 119 passed, +3 skipped`` over the two resolvers. Read it here rather than counting: three +distinct causes account for all eight, and only two of them are the provider's. + +*Six failures are the backend's flag set.* no released flagd-testbed seeds +``large-integer-flag`` nor ``integral-float-flag``, so the untagged precision +scenario, the ``max-int32`` row of the ``@variants`` outline and the lossless +``@numeric-coercion`` scenario fail ``FLAG_NOT_FOUND`` on each resolver alike. +open-feature/flagd-testbed#392 seeds all three of the flags the canonical set is +missing and says what each catches; bump the tag in ``docker-compose.yaml`` +beside this file when it lands. Left as failures rather than xfailed, because +they are true about the stack under test, and carrying no ``KnownDeviation`` in +either suite: the provider was never given the flag to get wrong. + +*One failure is ``openfeature-flagd-core``'s*, on in-process alone: +``boolean-flag`` requested as a Float resolves to ``1.0`` with reason ``STATIC`` +and no error code, where the mandatory wrong-type scenario asks for the caller's +default. ``bool`` is a subclass of ``int`` in Python and the int-to-float +widening does not exclude it. Filed as open-feature/python-sdk-contrib#417. RPC +passes the row, because the server type-checks it. No ``KnownDeviation``: the +scenario is mandatory and ungated, so it fails visibly on every run and an entry +would add nothing a reader cannot see. + +*One failure is flagd's*, on RPC alone, and it is the one failure here carrying a +``KnownDeviation``: ``float-flag`` (0.5) requested as an Integer comes back as +``0`` with no error code. **The in-process resolver passes that scenario**, which +is why the deviation is recorded against RPC only; ``test_rpc.py`` and +``test_in_process.py`` carry the measurement on each side. + +*The three skips are two scenarios.* ``@large-integers`` gates one and is +withheld on both resolvers, so it skips twice; ``@reinitialization`` gates one, +which in-process declares and passes and RPC withholds, so it skips once. Each +suite gives its own reason beside its declaration. + +**One finding came out of a scenario that passes**, so neither the results nor +the report has anywhere to put it: the in-process *Shutting down a provider that +cannot reach its backend completes promptly* returns well inside its bound and +leaves a ``PytestUnhandledThreadExceptionWarning`` behind it -- gRPC's +connectivity polling thread raising ``ValueError: Cannot invoke RPC: Channel +closed!`` after ``shutdown`` closed the channel underneath it. Reproducible on +every run, and filed as open-feature/python-sdk-contrib#419. Not a deviation: +nothing required is unmet. It is noted here because a reader who sees the warning +should know it is a recorded finding rather than noise. + +The canonical set's ``targeting-key-flag``, four ``disabled-*`` flags and three +falsy flags need no testbed change: they are flagd-testbed's own flags, which is +why the canonical set adopted their names and variants, and the launchpad's +``default`` configuration serves them as they stand. +""" + +from __future__ import annotations + +import socket +from pathlib import Path + +import pytest + +from openfeature.contrib.tools.tck import ComposeBackend, RunningBackend +from tests.tck.suite import IN_PROCESS_PORT, RPC_PORT + + +@pytest.fixture(scope="session") +def compose_backend() -> ComposeBackend: + """The stack under test, as the TCK's ``tck_backend`` fixture wants it. + + Both resolver ports are declared even though each suite uses one of them, + because both suites share this stack. The Compose file publishes flagd's + OFREP port as well, which nothing here asks for: it is the same file the + OFREP adoption uses, and a port nobody declares is neither waited on nor + looked up. + + The path is absolute so that pytest run from the repository root works too; + a relative one resolves against the working directory. + """ + return ComposeBackend( + compose_file=Path(__file__).parent / "docker-compose.yaml", + backend_ports=[RPC_PORT, IN_PROCESS_PORT], + ) + + +@pytest.fixture(scope="session") +def closed_port(tck_backend: RunningBackend) -> int: + """A port on localhost with nothing listening, for the ``@unavailable`` scenarios. + + Discovered by binding and releasing rather than hard-coded, because the + stack's own host ports are mapped dynamically and a fixed number could + collide with one; depending on ``tck_backend`` orders this after the stack + has taken its ports. Deliberately not a port on the stack, which has to stay + up -- simulated outages belong to the control API. + """ + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as probe: + probe.bind(("127.0.0.1", 0)) + return int(probe.getsockname()[1]) diff --git a/providers/openfeature-provider-flagd/tests/tck/docker-compose.yaml b/providers/openfeature-provider-flagd/tests/tck/docker-compose.yaml new file mode 100644 index 00000000..9ed476d5 --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/tck/docker-compose.yaml @@ -0,0 +1,20 @@ +# The backend every conformance suite in this repository runs against: the unmodified +# flagd testbed image, serving flagd and its launchpad control API. +# +# Not flagd-testbed's own compose file, which adds an envoy sidecar for its +# forbidden-endpoint scenarios, names its service `flagd`, and bind-mounts a flags +# directory the launchpad writes into. None of that is needed to drive the launchpad. +# +# This file exists twice, once per provider package, and the two are byte-identical: +# providers/openfeature-provider-flagd/tests/tck/docker-compose.yaml +# providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml +# Each package publishes its own distribution and must not read the other's files, so +# `diff` the two paths after changing either -- that diff is what catches drift. +services: + backend: + image: ghcr.io/open-feature/flagd-testbed:v3.10.1 + ports: + - 8013 # flagd RPC evaluation (gRPC) + - 8015 # flagd in-process sync (gRPC) + - 8016 # flagd's OFREP HTTP API + - 8080 # launchpad control API diff --git a/providers/openfeature-provider-flagd/tests/tck/suite.py b/providers/openfeature-provider-flagd/tests/tck/suite.py new file mode 100644 index 00000000..30e3d720 --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/tck/suite.py @@ -0,0 +1,197 @@ +"""Shared wiring for the two flagd conformance suites. + +flagd resolves flags two quite different ways -- RPC evaluates remotely over +gRPC, in-process syncs the ruleset and evaluates locally -- and they are separate +suites because they are separately conformant. Any difference between the two +results is a difference an application would see when it switches resolver, +which is exactly the class of thing the conformance suite exists to surface. + +Everything they share lives here; everything that differs lives in ``test_rpc.py`` +and ``test_in_process.py`` next to it, where a reader can see the whole of a +resolver's declaration in one place. The TCK owns the container stack (see +``conftest.py``), so what is left here is the timings, which are flagd's own and +interact, and the one function that turns a resolver plus a running endpoint into +a ``TckConfig``. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from openfeature.contrib.provider.flagd import FlagdProvider +from openfeature.contrib.provider.flagd.config import ResolverType +from openfeature.contrib.tools.tck import ( + Capability, + KnownDeviation, + RunningBackend, + TckConfig, +) +from openfeature.provider import FeatureProvider + +__all__ = ["IN_PROCESS_PORT", "RPC_PORT", "ResolverSuite", "build_config"] + +RPC_PORT = 8013 +IN_PROCESS_PORT = 8015 +"""Container-internal ports flagd serves remote evaluation and the sync stream on. + +Both are declared in one ``ComposeBackend`` because one flagd process serves both +and both suites share the stack. Named here rather than in ``conftest.py`` so +that the declaration and the resolver that uses it cannot drift apart. +""" + +# Timings. flagd exposes several and they interact, so they are named here once +# rather than scattered through two suites. + +DEADLINE_MS = 5000 +"""How long a provider blocks during initialisation before giving up. + +This, and not ``TckConfig.ready_timeout``, is what actually bounds flagd's +initialisation: the resolvers block inside ``initialize`` and raise +``ProviderNotReadyError`` when it expires (grpc.py:175, grpc_watcher.py:151). +Generous because every scenario is preceded by a control-API ``/start``, which +restarts the flagd process -- so a provider is routinely built against a backend +that came up milliseconds ago. + +For the RPC resolver it also bounds each individual resolution call. +""" + +RETRY_BACKOFF_MS = 500 +"""Initial delay before a reconnect attempt. Short, so an ended outage is noticed quickly.""" + +RETRY_BACKOFF_MAX_MS = 5000 +"""Longest delay between reconnect attempts, and the wait between stream retries. + +Kept at or above :data:`DEADLINE_MS`: the provider passes the deadline to gRPC as +``grpc.min_reconnect_backoff_ms`` and this as ``grpc.max_reconnect_backoff_ms`` +(grpc.py:90-92), so a smaller value here would be a channel configured with a +minimum backoff above its maximum. +""" + +RETRY_GRACE_PERIOD_SECONDS = 30 +"""How long a disconnected provider stays STALE before escalating to ERROR. + +Load-bearing for the ``@stale`` scenario. Both resolvers go STALE the moment the +channel fails and start a timer that escalates to ERROR when this expires +(grpc.py:204-210, grpc_watcher.py:180-186). Too short a value turns a scenario +about staleness into one about failure, because the outage lasts as long as the +scenario needs to observe it. +""" + +STREAM_DEADLINE_MS = 0 +"""Disable periodic stream recycling, as the existing e2e suites do. + +A recycle is invisible to the provider contract, but disabling it removes a +source of background reconnects from a suite whose whole subject is what a +reconnect looks like. +""" + +UNAVAILABLE_DEADLINE_MS = 500 +UNAVAILABLE_GRACE_PERIOD_SECONDS = 1 +UNAVAILABLE_BACKOFF_MS = 5000 +"""Deliberately impatient settings for the provider that cannot reach a backend. + +The ``@unavailable`` scenarios assert that failure is reported *promptly*, so a +provider taking 30 seconds to give up would pass a test about eventual failure +while failing the one that matters. The backoff is long for the opposite reason: +after the error is reported, the next failed reconnect attempt would emit +another ``PROVIDER_STALE`` and move the provider out of the ERROR state the +scenario is about to assert. +""" + +EVENT_TIMEOUT = 20.0 +"""Seconds to wait for a provider event. + +Comfortably above :data:`RETRY_BACKOFF_MAX_MS`, which is how long a provider may +wait before the reconnect attempt that produces the ``PROVIDER_READY`` ending +the ``@stale`` scenario. +""" + + +@dataclass(frozen=True) +class ResolverSuite: + """What differs between the two resolvers' suites.""" + + name: str + """Names the suite in test output and scopes its OpenFeature domain.""" + + resolver_type: ResolverType + + backend_port: int + """The container-internal port this resolver connects to. + + Resolved to a host port through the endpoint, after the stack is up. + """ + + capabilities: frozenset[Capability] + """What this resolver was run against the suite and seen to satisfy. + + See each suite module for the evidence behind every entry, and behind every + omission. + """ + + ready_timeout: float + + known_deviations: tuple[KnownDeviation, ...] = () + """Requirements this resolver is known to fail, acknowledged rather than hidden. + + **Per resolver, and that is the whole reason this field is here rather than + shared.** The two resolvers do not deviate alike: RPC narrows ``float-flag`` + to ``0`` where in-process refuses it, so an entry naming + :attr:`~Capability.NUMERIC_COERCION` is true of one and false of the other, + and attaching it to both would publish a defect against the resolver that + does not have it. + """ + + +def build_config( + suite: ResolverSuite, + backend: RunningBackend, + closed_port: int, +) -> TckConfig: + """Wire one resolver up to the running testbed. + + The host and port are read off ``backend.endpoint`` inside the factory, which + is why the factory exists: the mapped host port does not exist until the + stack is up. The host comes from the endpoint rather than being written as + ``"localhost"``, because with a remote Docker daemon, Docker Desktop on some + platforms or a rootless setup it is neither localhost nor predictable. + """ + endpoint = backend.endpoint + + def new_provider() -> FeatureProvider: + return FlagdProvider( + resolver_type=suite.resolver_type, + host=endpoint.host, + port=endpoint.port(suite.backend_port), + deadline_ms=DEADLINE_MS, + stream_deadline_ms=STREAM_DEADLINE_MS, + retry_backoff_ms=RETRY_BACKOFF_MS, + retry_backoff_max_ms=RETRY_BACKOFF_MAX_MS, + retry_grace_period=RETRY_GRACE_PERIOD_SECONDS, + ) + + def new_unavailable_provider() -> FeatureProvider: + # Pointed at a closed port on localhost, never at the backend under + # test: that has to stay up, and simulated outages belong to the control + # API. + return FlagdProvider( + resolver_type=suite.resolver_type, + host="localhost", + port=closed_port, + deadline_ms=UNAVAILABLE_DEADLINE_MS, + stream_deadline_ms=STREAM_DEADLINE_MS, + retry_backoff_ms=UNAVAILABLE_BACKOFF_MS, + retry_backoff_max_ms=UNAVAILABLE_BACKOFF_MS, + retry_grace_period=UNAVAILABLE_GRACE_PERIOD_SECONDS, + ) + + return TckConfig( + name=suite.name, + control=backend.control, + new_provider=new_provider, + new_unavailable_provider=new_unavailable_provider, + capabilities=suite.capabilities, + known_deviations=suite.known_deviations, + event_timeout=EVENT_TIMEOUT, + ready_timeout=suite.ready_timeout, + ) diff --git a/providers/openfeature-provider-flagd/tests/tck/test_in_process.py b/providers/openfeature-provider-flagd/tests/tck/test_in_process.py new file mode 100644 index 00000000..43360bff --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/tck/test_in_process.py @@ -0,0 +1,228 @@ +"""The OpenFeature provider conformance suite, run against flagd's in-process resolver. + +In-process syncs the whole ruleset over flagd's sync API and evaluates locally, +so unlike RPC the type-checking, the variant selection and the reason all come +from ``openfeature-flagd-core`` in this process rather than from the server. Any +difference in the results is a difference an application would see when it +switches resolver, which is why this is a separate suite rather than a +parametrisation of the RPC one. +""" + +from __future__ import annotations + +import pytest +from pytest_bdd import scenarios + +from openfeature.contrib.provider.flagd.config import ResolverType +from openfeature.contrib.tools.tck import ( + Capability, + RunningBackend, + TckConfig, + feature_paths, +) +from tests.tck.suite import IN_PROCESS_PORT, ResolverSuite, build_config + +# Every capability below was declared, the suite run, and the scenarios seen to +# pass. The code references say where the behaviour lives, so that a reader can +# check the claim -- the run is what it rests on, which is Appendix F's rule and +# not a preference here. @reinitialization below shows why from the other side: +# this resolver does support reuse, and reading it does not say so. +# +# EVENTS grpc_watcher.py:262 emits PROVIDER_READY once the first sync +# payload has been *applied*, not merely received: the ruleset is +# written to the evaluator at grpc_watcher.py:254 first, so a +# scenario evaluating straight after ready cannot race the sync. +# STALE grpc_watcher.py:178-190 emits PROVIDER_STALE on TRANSIENT_FAILURE +# and starts the timer escalating to PROVIDER_ERROR only once +# retry_grace_period expires. +# CONFIGURATION_CHANGE +# in_process.py:34 emits PROVIDER_CONFIGURATION_CHANGED naming +# exactly the keys FlagdCore reports as changed, from every sync +# payload the watcher applies. +# OBJECT in_process.py:122 resolves structured values from the local +# ruleset. +# VARIANTS flagd_core.py returns the variant it selected with every +# resolution and in_process.py carries it into the details; the +# ruleset is keyed by variant, so there is always one to report. +# TARGETING targeting.py:40-41 puts the evaluation context's targeting key +# into the JSON-logic context under `targetingKey` and +# flagd_core.py:154 evaluates the rule against it, so +# targeting-key-flag selects hit or miss locally. +# UNAVAILABLE_INIT +# grpc_watcher.py:151 raises ProviderNotReadyError once the +# blocking init deadline passes without a synced ruleset, which the +# SDK's registry turns into PROVIDER_ERROR. +# +# DISABLED_FLAGS +# All four rows pass, and here that is the unsurprising half of the story: +# evaluation is local, so the resolver has the flag's state and the caller's +# default in the same call. flagd_core.py:143-145 returns the caller's +# `default_value` with reason Reason.DISABLED the moment a flag's state is +# DISABLED, before any targeting or variant selection. flagd_core.py:199-200 +# then skips the type check for that reason, which is what stops the +# substituted default from being re-typed against the flag it did not come +# from. +# +# Probed directly, each of the four flags resolves to the caller's default +# with reason Reason.DISABLED, no variant and no error code -- the SDK's +# enum, where RPC hands back the server's bare 'DISABLED' string. Nothing +# asserts either, so the difference is recorded rather than acted on; it is +# the sort of divergence between the two resolvers this pair of suites exists +# to surface. +# +# LIFECYCLE +# All six scenarios execute and all six pass. +# +# It is a real question here rather than a formality, which is the test the +# capability's own documentation sets: an SDK synthesises PROVIDER_READY +# around `initialize` for any provider, so the readiness scenario is vacuous +# for a provider that does nothing during initialisation. This resolver syncs +# the entire ruleset and applies it to the evaluator before ready is emitted +# (grpc_watcher.py:254-262), and initialisation can and does fail +# (grpc_watcher.py:151), so both terminal outcomes the feature file asserts +# are outcomes this provider actually reaches. +# +# Running them also produced a finding neither a pass nor a fail carries -- +# a shutdown-ordering race that leaves a stray traceback behind a *passing* +# scenario. conftest.py records it; open-feature/python-sdk-contrib#419. +# +# NUMERIC_COERCION +# Declared here and declared on RPC too, but they are not the same claim, +# and that is the most interesting thing in this pair of suites: **this +# resolver satisfies the lossy half and RPC does not.** Measured, both +# resolvers, same run -- `float-flag` (0.5) requested as an Integer is a +# TYPE_MISMATCH returning the caller's default here, and comes back as `0` +# with no error code at all on RPC. One provider, two resolvers, opposite +# answers to the question the capability exists to ask +# (open-feature/python-sdk-contrib#420). +# +# Why it holds here: evaluation is local, so flagd_core.py:25 admits only +# `int` for an integer request and `_check_type` (flagd_core.py:228-231) +# raises TypeMismatchError for anything else. The float mapping at +# flagd_core.py:26 is the wider `(int, float)` and `resolve_float_value` +# (flagd_core.py:113-114) widens an int result, so `integer-flag` requested +# as a Float is 10.0. Both of the scenarios this backend can put to the +# provider pass, and no KnownDeviation is recorded against this resolver -- +# there is no defect here to record. The entry on the RPC suite is +# deliberately not mirrored onto this one. +# +# The tag's third scenario fails, and not for a reason about coercion: +# flagd-testbed seeds no `integral-float-flag`, so it fails FLAG_NOT_FOUND. +# What this resolver would do with a seeded 10.0 is still unmeasured -- the +# `(int,)` rule above says it would refuse it, but that is a reading of the +# source and nothing here observes it. Withholding the tag over the one +# scenario this backend cannot ask would discard the finding that the two +# resolvers differ. +# +# STRING_TYPING and FULLY_TYPED_VALUES +# Both declared, on a run: all four scenarios pass on this resolver -- two +# Examples rows asking for `boolean-flag` and `integer-flag` through the +# string accessor, and the `float-flag` and `object-flag` scenarios that +# spec bda599f1 moved behind the second tag. +# +# Here the type check is local and readable, which is the contrast with RPC. +# `_TYPE_MAP`'s string entry is the narrow `(str,)` (flagd_core.py:23) and +# `_check_type` (flagd_core.py:228-231) raises TypeMismatchError for +# anything else, so nothing renders a bool, a number or a structure as text +# on the way out. Note that this is the one accessor `bool` does not slip +# through: the int-to-float widening that makes `boolean-flag` as a Float +# resolve to 1.0 (python-sdk-contrib#417, the ungated failure below) has no +# counterpart here, because `str` is not in bool's ancestry. +# +# Both tags rather than one, and here the reason is the same table: +# `_TYPE_MAP` (flagd_core.py:22-28) gives `float` `(int, float)` and +# `object` `(dict, list)`, so this resolver holds a float as a number and a +# structure as a dict or list, exactly as it holds a bool as a bool. There +# is no type the ruleset keeps as text -- it is `json.loads` output all the +# way down (flagd_core.py:73) -- so the question @fully-typed-values asks has +# an answer here. The split is for a backend that types booleans and +# integers and nothing else, and this is not one. +# +# Four scenarios that were mandatory before spec d47a66eb, and passing, so +# the two declarations keep them running and change no number: the full run +# is 8 failed, 119 passed, 3 skipped before and after. +# +# REINITIALIZATION +# Declared here and withheld on RPC, and again the two resolvers genuinely +# differ: this one shuts down and starts again serving `boolean-flag` +# correctly, while RPC evaluates against a closed channel. Measured on the +# same run. Requirement 2.5.2 permits reuse rather than requiring it, which +# is why RPC's withholding needs no KnownDeviation and why declaring it here +# is a claim worth making rather than a box ticked. +# +# STANDARD_REASONS +# Declared on a run rather than on the source. All nine scenarios pass -- the +# four rule-less rows as STATIC, an unknown flag and a type mismatch as ERROR +# beside their error codes, TARGETING_MATCH for the matched rule and DEFAULT +# for the miss, DISABLED for a disabled flag. The last three need @targeting +# and @disabled-flags as well, which this suite declares, so none of the file +# is skipped here. +# +# The DISABLED row resolves through Reason.DISABLED here, the SDK's own enum, +# where RPC hands back flagd's bare 'DISABLED' string -- noted under +# DISABLED_FLAGS above. The step compares the reason as text, so both pass. +# +# Not declared, and why: +# +# LARGE_INTEGERS +# Withheld. Exactly one scenario carries the tag, it asks for +# `huge-integer-flag`, and no released flagd-testbed seeds such a flag -- so this +# backend can put none of the tag's scenarios to this provider, and Appendix +# F's sixth declaring rule says withhold. Python's `int` is unbounded and the +# ruleset arrives as JSON text parsed with `json.loads` (flagd_core.py:73), +# so nothing here would narrow the value -- and the suite cannot show that, +# which is the point. +# +# Contrast @numeric-coercion above, which is a fixture gap too and is +# declared: two of its three scenarios do reach the provider, and the two +# resolvers answer them differently. The rule counts scenarios, not tags. +# +# No KnownDeviation, in either shape: the gap is the backend's and an entry +# would attribute it to the provider. This withholding is temporary in a way +# the ones above are not -- open-feature/flagd-testbed#392 adds the flag; +# declare the tag when the image carries it, or it outlives its reason and +# starts reading as a claim about the provider. +# +# CACHING +# Reserved, and the harness refuses it: no scenario carries the tag. +IN_PROCESS_CAPABILITIES = frozenset( + { + Capability.EVENTS, + Capability.STALE, + Capability.CONFIGURATION_CHANGE, + Capability.OBJECT, + Capability.VARIANTS, + Capability.DISABLED_FLAGS, + Capability.TARGETING, + Capability.UNAVAILABLE_INIT, + Capability.STANDARD_REASONS, + Capability.LIFECYCLE, + Capability.NUMERIC_COERCION, + Capability.STRING_TYPING, + Capability.FULLY_TYPED_VALUES, + Capability.REINITIALIZATION, + } +) + +IN_PROCESS_SUITE = ResolverSuite( + name="flagd-in-process", + resolver_type=ResolverType.IN_PROCESS, + backend_port=IN_PROCESS_PORT, + capabilities=IN_PROCESS_CAPABILITIES, + # In-process transfers and applies the whole ruleset before reporting ready, + # so it needs more headroom than RPC. + ready_timeout=60.0, +) + + +@pytest.fixture(scope="session") +def tck_config(tck_backend: RunningBackend, closed_port: int) -> TckConfig: + """The whole of this adoption's wiring. + + ``tck_backend`` is the TCK's own session-scoped fixture: the stack is up and + its control is ready by the time this runs. + """ + return build_config(IN_PROCESS_SUITE, tck_backend, closed_port) + + +scenarios(*feature_paths()) diff --git a/providers/openfeature-provider-flagd/tests/tck/test_rpc.py b/providers/openfeature-provider-flagd/tests/tck/test_rpc.py new file mode 100644 index 00000000..49f4903f --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/tck/test_rpc.py @@ -0,0 +1,262 @@ +"""The OpenFeature provider conformance suite, run against flagd's RPC resolver. + +RPC asks flagd to evaluate each flag over gRPC and maps the response onto typed +resolution details, so what is under test here is that mapping plus the +lifecycle the evaluation stream drives. +""" + +from __future__ import annotations + +import pytest +from pytest_bdd import scenarios + +from openfeature.contrib.provider.flagd.config import ResolverType +from openfeature.contrib.tools.tck import ( + Capability, + KnownDeviation, + RunningBackend, + TckConfig, + feature_paths, +) +from tests.tck.suite import RPC_PORT, ResolverSuite, build_config + +# Every capability below was declared, the suite run, and the scenarios seen to +# pass. The code references say where the behaviour lives, so that a reader can +# check the claim -- the run is what it rests on, which is Appendix F's rule and +# not a preference here. @reinitialization is the case in point: this resolver's +# shutdown reverts enough of its own state to read as support for reuse, and the +# channel underneath cannot be rebuilt. +# +# EVENTS grpc.py:261 emits PROVIDER_READY when the evaluation stream +# delivers its 'provider_ready' message. +# STALE grpc.py:202-212 emits PROVIDER_STALE on TRANSIENT_FAILURE and +# only then starts the timer escalating to PROVIDER_ERROR once +# retry_grace_period expires. +# CONFIGURATION_CHANGE +# grpc.py:302 emits PROVIDER_CONFIGURATION_CHANGED with the changed +# keys, and grpc.py:298-300 evicts exactly those keys from the LRU +# cache -- so the re-evaluation the scenario performs afterwards +# cannot be served a stale cached value. +# OBJECT grpc.py:336 resolves structured values through ResolveObject. +# VARIANTS grpc.py:449 carries the response's `variant` into the resolution +# details for every typed call. Seven of the outline's eight rows +# pass; the eighth asks for a flag the backend does not seed. +# TARGETING grpc.py:492 puts the evaluation context's targeting key into the +# request's context struct, so the server evaluates +# targeting-key-flag's rule against it and answers hit or miss. +# UNAVAILABLE_INIT +# grpc.py:175 raises ProviderNotReadyError once the blocking init +# deadline passes without a connection, which the SDK's registry +# turns into PROVIDER_ERROR. +# +# DISABLED_FLAGS +# All four rows pass, and the measurement is worth keeping because a remote +# evaluator satisfying this is not obvious. flagd answers a disabled flag +# with reason DISABLED, no variant, and the zero value of the response proto +# -- ResolveBoolean's `value` field is simply unset -- and grpc.py:468-472 +# replaces that with the caller's `default_value` whenever the reason is +# DEFAULT or DISABLED and no variant came back. So the substitution is local +# even though the evaluation is not: what crosses the wire is the signal, and +# the provider already holds the default. +# +# Probed directly, each of the four flags resolves to the caller's default +# with reason 'DISABLED', no variant and no error code. The reason arrives as +# the server's bare string rather than the SDK's Reason enum, which nothing +# asserts -- noted only because the in-process resolver differs, returning +# Reason.DISABLED. +# +# LIFECYCLE +# All six scenarios execute, five pass, and the sixth is the +# @reinitialization one dealt with below. +# +# It is a real question here rather than a formality, which is the test the +# capability's own documentation sets: an SDK synthesises PROVIDER_READY +# around `initialize` for any provider, so the readiness scenario is vacuous +# for one that does nothing during initialisation. This resolver blocks until +# the evaluation stream is up and raises ProviderNotReadyError when the +# deadline passes (grpc.py:175, grpc.py:261), so both terminal outcomes the +# feature file asserts are outcomes this provider actually reaches. +# +# NUMERIC_COERCION +# Declared, and failing the lossy scenario -- deliberately, and the failure +# is the report. See KNOWN_DEVIATIONS below. +# +# RPC does not type-check locally: grpc.py:444-448 asks flagd for an Int and +# passes back whatever the server answers, so the whole decision is flagd's. +# flagd's evaluator resolves the variant as a float64 and casts it with a +# bare `int64(val)` (core/pkg/evaluator/json.go, ResolveIntValue, at the +# v0.16.0 the testbed's `flagd/Dockerfile` builds on), so `float-flag`'s 0.5 +# comes back as 0 with reason STATIC and no error code -- silently narrowed, +# which is the one thing the lossy scenario forbids. Measured, not read off +# the server source: the scenario fails with `flag 'float-flag' resolved to +# 0 (int), expected 1 (int)`. +# +# **The in-process resolver passes that scenario.** It refuses 0.5 as an +# Integer with TYPE_MISMATCH, because it evaluates locally and never asks +# flagd, while `integer-flag` requested as a Float widens correctly on both. +# One provider, two resolvers, opposite answers to the question this +# capability exists to ask -- which is why the deviation below is on this +# suite only and is deliberately not mirrored onto that one, and why the tag +# is declared and left to fail rather than withheld: a skip cannot say "it +# coerces, and one direction is wrong". Filed as +# open-feature/python-sdk-contrib#420. +# +# The tag's third scenario fails for a reason that is not flagd's: +# flagd-testbed seeds no `integral-float-flag`, so it is FLAG_NOT_FOUND. The +# conftest records it and the deviation summary disclaims it; it is not a +# reason to withhold the tag. +# +# STRING_TYPING and FULLY_TYPED_VALUES +# Both declared, on a run: all four scenarios pass on this resolver -- two +# Examples rows asking for `boolean-flag` and `integer-flag` through the +# string accessor, and the `float-flag` and `object-flag` scenarios that +# spec bda599f1 moved behind the second tag. +# +# Declared rather than assumed, because the question is about the backend +# rather than about the provider and RPC is where the backend answers it. +# The decision is flagd's for the same reason the coercion one is: grpc.py +# asks the server for a String and hands back what it answers, and flagd +# types each variant from the JSON ruleset, so a bool or a number reached +# through ResolveString is a TYPE_MISMATCH the server raises rather than one +# the provider notices. That is the property a string-storing backend lacks +# -- see the capability's docstring and Appendix F's `@string-typing` +# section -- and it is not a property of flagd that could be read off this +# repository at all. +# +# Which is the same answer for the second tag, and why both are declared +# rather than one. flagd holds a ruleset, not a string table, and it types +# every variant out of the JSON: a float variant is a float and a structured +# one is an object, so there is no type it keeps as text and every one of the +# four questions has an answer here. The split exists for a backend that +# records booleans and integers natively and nothing else -- such a provider +# declares @string-typing and withholds @fully-typed-values, and the measured +# reason it exists is that one tag over all four cases published a real +# defect in another language as a permitted absence. flagd is on the other +# side of that line, and declaring both is what says so. +# +# Four scenarios that were mandatory before spec d47a66eb, and passing, so +# the two declarations keep them running and change no number: the full run +# is 8 failed, 119 passed, 3 skipped before and after. +# +# STANDARD_REASONS +# Declared on a run rather than on the source. All nine scenarios pass -- the +# four rule-less rows as STATIC, an unknown flag and a type mismatch as ERROR +# beside their error codes, TARGETING_MATCH for the matched rule and DEFAULT +# for the miss, DISABLED for a disabled flag. The last three need @targeting +# and @disabled-flags as well, which this suite declares, so none of the file +# is skipped here. +# +# The DISABLED row passes despite the reason arriving as flagd's bare string +# rather than the SDK's Reason enum: the step compares the reason as text. +# +# Not declared, and why: +# +# LARGE_INTEGERS +# Withheld. Exactly one scenario carries the tag, it asks for +# `huge-integer-flag`, and no released flagd-testbed seeds such a flag -- so this +# backend can put none of the tag's scenarios to this provider, and Appendix +# F's sixth declaring rule says withhold. Nothing in this path would narrow +# the value: flagd holds every numeric variant as a float64 and 2^53 - 1 is +# exactly the largest integer a float64 represents without rounding, the +# server casts it to the int64 of ResolveIntResponse.value, and grpc.py:448 +# hands that to the SDK as an unbounded Python int. Nothing in between is 32 +# bits wide -- and the suite cannot show that, which is the point. +# +# Contrast @numeric-coercion above, which is a fixture gap too and is +# declared: two of its three scenarios do reach the provider, and the two +# resolvers answer them differently. The rule counts scenarios, not tags. +# +# No KnownDeviation, in either shape: the gap is the backend's and an entry +# would attribute it to the provider. This withholding is temporary in a way +# the ones above are not -- open-feature/flagd-testbed#392 adds the flag; +# declare the tag when the image carries it, or it outlives its reason and +# starts reading as a claim about the provider. +# +# REINITIALIZATION +# Withheld, on the measurement: with LIFECYCLE declared the scenario runs, +# and it fails with `boolean-flag` resolving to the code default because +# grpc.py:420 raises "Cannot invoke RPC on closed channel!". shutdown() +# closes the channel and the second initialize() does not rebuild it, so the +# provider evaluates against a closed connection rather than failing +# outright. Requirement 2.5.2 permits reuse rather than requiring it, so this +# is a choice the specification offers and there is no requirement to deviate +# from: no KnownDeviation. The in-process suite declares it for the same +# reason in reverse -- it runs, and it passes. +# +# CACHING +# Reserved, and the harness refuses it: no scenario carries the tag. +RPC_CAPABILITIES = frozenset( + { + Capability.EVENTS, + Capability.STALE, + Capability.CONFIGURATION_CHANGE, + Capability.OBJECT, + Capability.VARIANTS, + Capability.DISABLED_FLAGS, + Capability.TARGETING, + Capability.UNAVAILABLE_INIT, + Capability.STANDARD_REASONS, + Capability.LIFECYCLE, + Capability.NUMERIC_COERCION, + Capability.STRING_TYPING, + Capability.FULLY_TYPED_VALUES, + } +) + +KNOWN_DEVIATIONS = ( + KnownDeviation.tracked( + capability=Capability.NUMERIC_COERCION, + issue="https://github.com/open-feature/flagd/issues/1996", + summary=( + "The lossy half of the coercion rule is not enforced: evaluating " + "float-flag (0.5) through the integer accessor returns 0 with no " + "error code, rather than TYPE_MISMATCH with the code default, so " + "the fractional part is discarded silently. Lossless coercion is " + "permitted and is not the defect -- this resolver does widen an " + "integer to a float correctly, which is why the capability is " + "declared and the scenario left to fail rather than the capability " + "withheld. The rule is flagd's own accepted numeric-coercion ADR " + "rather than a specification requirement, which does not define " + "numeric coercion at all (open-feature/spec#430), so this is a " + "deviation from a commitment flagd made rather than from the " + "provider contract. Unlike the Java and Go flagd providers, whose " + "two resolvers narrow identically and which therefore place the " + "defect in their shared provider layer, this one is in the server " + "alone: the Python in-process resolver evaluates locally and " + "refuses 0.5 as an integer correctly, so it is not recorded as " + "deviating and carries no equivalent entry. The " + "tag's third scenario also fails, but for an unrelated reason that " + "is not flagd's: integral-float-flag is absent from the pinned " + "flagd-testbed image, open-feature/flagd-testbed#392." + ), + ), +) +"""The one requirement this resolver is known to fail. + +Declared-and-failing rather than withheld-and-skipped, and recorded against this +resolver only: the in-process suite passes the scenario this deviates on. +""" + +RPC_SUITE = ResolverSuite( + name="flagd-rpc", + resolver_type=ResolverType.RPC, + backend_port=RPC_PORT, + capabilities=RPC_CAPABILITIES, + known_deviations=KNOWN_DEVIATIONS, + # RPC holds no ruleset of its own: it is ready as soon as the evaluation + # stream is up, so it needs less headroom than in-process. + ready_timeout=30.0, +) + + +@pytest.fixture(scope="session") +def tck_config(tck_backend: RunningBackend, closed_port: int) -> TckConfig: + """The whole of this adoption's wiring. + + ``tck_backend`` is the TCK's own session-scoped fixture: the stack is up and + its control is ready by the time this runs. + """ + return build_config(RPC_SUITE, tck_backend, closed_port) + + +scenarios(*feature_paths()) diff --git a/uv.lock b/uv.lock index ea025adb..8017df40 100644 --- a/uv.lock +++ b/uv.lock @@ -1886,6 +1886,7 @@ dev = [ { name = "coverage", extra = ["toml"] }, { name = "grpcio-health-checking" }, { name = "mypy" }, + { name = "openfeature-tck", extra = ["compose"] }, { name = "poethepoet" }, { name = "pytest" }, { name = "pytest-bdd" }, @@ -1911,6 +1912,7 @@ dev = [ { name = "coverage", extras = ["toml"], specifier = ">=7.10.0,<8.0.0" }, { name = "grpcio-health-checking", specifier = ">=1.82.1,<2.0.0" }, { name = "mypy", specifier = ">=1.18.0,<2.0.0" }, + { name = "openfeature-tck", extras = ["compose"], editable = "tools/openfeature-tck" }, { name = "poethepoet", specifier = ">=0.37.0" }, { name = "pytest", specifier = ">=9.0.0,<10.0.0" }, { name = "pytest-bdd", specifier = ">=8.1.0,<9.0.0" },