diff --git a/providers/openfeature-provider-flagd/pyproject.toml b/providers/openfeature-provider-flagd/pyproject.toml index 2c1c1b3a..7cb0492d 100644 --- a/providers/openfeature-provider-flagd/pyproject.toml +++ b/providers/openfeature-provider-flagd/pyproject.toml @@ -35,6 +35,10 @@ 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 provider conformance suite. Ships the feature files, the flag + # set and the control-API client, and registers its step definitions through a + # pytest11 entry point, so tests/tck needs no conftest of its own for them. + "openfeature-provider-tck", "poethepoet>=0.37.0", "pytest>=9.0.0,<10.0.0", "pytest-bdd>=8.1.0,<9.0.0", @@ -112,6 +116,7 @@ warn_unused_ignores = false [tool.uv.sources] openfeature-flagd-core = { workspace = true } +openfeature-provider-tck = { workspace = true } [tool.pytest] strict = true 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..b67feaaa --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/tck/conftest.py @@ -0,0 +1,73 @@ +"""One testbed stack and one control, shared by both conformance suites. + +The stack is started once per session and **never restarted**. Scenario +isolation comes from the control API instead, because container orchestrators +assign host ports dynamically and cannot reliably preserve them across a +restart: a restarted backend comes back on a different host port, silently +invalidating every provider already pointed at the old one, and the failure +looks like a flaky provider rather than a broken test. See the +no-container-restart invariant in the TCK's ``control-api.yaml``. + +``flagd-testbed`` is not modified and the existing e2e suites are untouched: the +TCK drives the testbed's launchpad through the standardised control API, which +the launchpad already implements, and reuses the container lifecycle already in +``tests/e2e``. +""" + +from __future__ import annotations + +import socket +import typing + +import pytest + +from openfeature.contrib.tools.provider_tck import HttpControl +from tests.e2e.flagd_container import FlagdContainer + + +@pytest.fixture(scope="session") +def flagd_testbed() -> typing.Iterator[FlagdContainer]: + """The testbed stack, up for the whole session. + + One stack for both suites because one flagd process serves both ports the + resolvers use -- 8013 for RPC and 8015 for sync -- so there is nothing a + second stack would isolate. + """ + container = FlagdContainer() + container.start() + try: + yield container + finally: + container.stop() + + +@pytest.fixture(scope="session") +def flagd_control(flagd_testbed: FlagdContainer) -> HttpControl: + """The control API client, shared by both suites. + + Shared rather than one per suite, and that matters: the two suites drive the + *same* backend, and ``HttpControl`` tracks whether a disconnect has left it + down so the next scenario starts it rather than merely resetting flag state. + Two instances would each hold half of that knowledge. + + The launchpad registers only ``/start``, ``/restart``, ``/stop`` and + ``/change`` (flagd-testbed ``launchpad/main.go``), so ``/reset`` answers 404 + and every ``prepare_scenario`` takes the documented ``/start`` fallback. The + probe costs one 404 for the whole session. + """ + return HttpControl(flagd_testbed.get_launchpad_url()) + + +@pytest.fixture(scope="session") +def closed_port(flagd_testbed: FlagdContainer) -> int: + """A port on localhost with nothing listening, for the ``@unavailable`` scenarios. + + Discovered by binding and releasing rather than hard-coded, because the + testbed's own host ports are mapped dynamically and a hard-coded number + could collide with one. Depending on ``flagd_testbed`` orders this after the + stack has taken its ports, which is what makes the remaining race + negligible. + """ + 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/suite.py b/providers/openfeature-provider-flagd/tests/tck/suite.py new file mode 100644 index 00000000..8a3c3e35 --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/tck/suite.py @@ -0,0 +1,167 @@ +"""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 the two +``test_*_conformance`` modules next to it, where a reader can see the whole of a +resolver's declaration in one place. +""" + +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.provider_tck import ( + Capability, + HttpControl, + TckConfig, +) +from openfeature.provider import FeatureProvider +from tests.e2e.flagd_container import FlagdContainer + +__all__ = ["ResolverSuite", "build_config"] + +# 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 + + capabilities: frozenset[Capability] + """Derived from reading the resolver's event emission, not from running the suite. + + See each suite module for the evidence behind every entry, and behind every + omission. + """ + + ready_timeout: float + + +def build_config( + suite: ResolverSuite, + container: FlagdContainer, + control: HttpControl, + closed_port: int, +) -> TckConfig: + """Wire one resolver up to the running testbed. + + The ports are read here, after the stack is up: the testbed maps host ports + dynamically, so they do not exist earlier -- and they stay valid for the + whole session because nothing ever restarts a container. Outages are + simulated inside the running stack through ``control`` instead. + """ + port = container.get_port(suite.resolver_type) + + def new_provider() -> FeatureProvider: + return FlagdProvider( + resolver_type=suite.resolver_type, + host="localhost", + port=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=control, + new_provider=new_provider, + new_unavailable_provider=new_unavailable_provider, + capabilities=suite.capabilities, + event_timeout=EVENT_TIMEOUT, + ready_timeout=suite.ready_timeout, + ) diff --git a/providers/openfeature-provider-flagd/tests/tck/test_in_process_conformance.py b/providers/openfeature-provider-flagd/tests/tck/test_in_process_conformance.py new file mode 100644 index 00000000..b2a79822 --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/tck/test_in_process_conformance.py @@ -0,0 +1,96 @@ +"""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.provider_tck import ( + Capability, + HttpControl, + TckConfig, + features_path, +) +from tests.e2e.flagd_container import FlagdContainer +from tests.tck.suite import ResolverSuite, build_config + +# Every capability below is declared on the strength of a line of provider code, +# not on the strength of a green run. +# +# EVENTS +# grpc_watcher.py:262 emits PROVIDER_READY once the first sync payload has +# been applied -- note "applied", not "received": the ruleset is written to +# the evaluator at grpc_watcher.py:254 before ready is emitted, so a scenario +# that evaluates immediately after ready cannot race the first sync. +# +# STALE +# grpc_watcher.py:178-190: the channel-connectivity callback emits +# PROVIDER_STALE on TRANSIENT_FAILURE and starts a timer that escalates to +# PROVIDER_ERROR only once retry_grace_period expires. +# +# CONFIGURATION_CHANGE +# in_process.py:34 emits PROVIDER_CONFIGURATION_CHANGED naming exactly the +# keys that FlagdCore reports as changed, from every sync payload the watcher +# applies. +# +# OBJECT +# in_process.py:122 resolves structured values from the local ruleset. +# +# 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. +# +# STRICT_NUMERIC_TYPING +# Local, and strict: flagd_core.py:25 admits only `int` for an integer +# request, and flagd_core.py:228-231 raises TypeMismatchError for anything +# else -- so `float-flag`'s 0.5 is reported as a mismatch rather than +# narrowed to 0. (The float mapping at flagd_core.py:26 is deliberately the +# wider one, `(int, float)`, but widening towards float loses nothing.) +# +# Not declared, and why: +# +# TARGETING, CACHING +# Reserved in the Capability enum; no scenario carries either tag. Declaring +# a capability nothing exercises would be a claim with no evidence behind it, +# so they are left out of both suites. +IN_PROCESS_CAPABILITIES = frozenset( + { + Capability.EVENTS, + Capability.STALE, + Capability.CONFIGURATION_CHANGE, + Capability.OBJECT, + Capability.UNAVAILABLE_INIT, + Capability.STRICT_NUMERIC_TYPING, + } +) + +IN_PROCESS_SUITE = ResolverSuite( + name="flagd-in-process", + resolver_type=ResolverType.IN_PROCESS, + 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( + flagd_testbed: FlagdContainer, + flagd_control: HttpControl, + closed_port: int, +) -> TckConfig: + return build_config(IN_PROCESS_SUITE, flagd_testbed, flagd_control, closed_port) + + +scenarios(features_path()) diff --git a/providers/openfeature-provider-flagd/tests/tck/test_rpc_conformance.py b/providers/openfeature-provider-flagd/tests/tck/test_rpc_conformance.py new file mode 100644 index 00000000..5aa38982 --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/tck/test_rpc_conformance.py @@ -0,0 +1,97 @@ +"""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.provider_tck import ( + Capability, + HttpControl, + TckConfig, + features_path, +) +from tests.e2e.flagd_container import FlagdContainer +from tests.tck.suite import ResolverSuite, build_config + +# Every capability below is declared on the strength of a line of provider code, +# not on the strength of a green run. +# +# EVENTS +# grpc.py:261 emits PROVIDER_READY when the evaluation stream delivers its +# 'provider_ready' message. +# +# STALE +# grpc.py:202-212: the channel-connectivity callback emits PROVIDER_STALE on +# TRANSIENT_FAILURE and only then starts a timer that escalates to +# PROVIDER_ERROR once retry_grace_period expires. +# +# Worth calling out, because the Go provider does NOT do this: its RPC +# resolver sends ProviderError directly on connection loss and never emits +# PROVIDER_STALE, which is filed as go-sdk-contrib#939 and is why the Go +# adoption withholds this capability for RPC. Python has no such asymmetry -- +# both of its resolvers share the same state-change callback shape -- so the +# capability is declared here. +# +# 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. +# +# 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. +# +# STRICT_NUMERIC_TYPING +# RPC does not type-check locally; it asks flagd for an Int and flagd answers +# INVALID_ARGUMENT for a float-valued flag, which grpc.py:461-462 maps to +# TypeMismatchError. So 0.5 is never narrowed to 0. +# +# Not declared, and why: +# +# TARGETING, CACHING +# Reserved in the Capability enum; no scenario carries either tag. Declaring +# a capability nothing exercises would be a claim with no evidence behind it, +# so they are left out of both suites. +RPC_CAPABILITIES = frozenset( + { + Capability.EVENTS, + Capability.STALE, + Capability.CONFIGURATION_CHANGE, + Capability.OBJECT, + Capability.UNAVAILABLE_INIT, + Capability.STRICT_NUMERIC_TYPING, + } +) + +RPC_SUITE = ResolverSuite( + name="flagd-rpc", + resolver_type=ResolverType.RPC, + capabilities=RPC_CAPABILITIES, + # 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( + flagd_testbed: FlagdContainer, + flagd_control: HttpControl, + closed_port: int, +) -> TckConfig: + return build_config(RPC_SUITE, flagd_testbed, flagd_control, closed_port) + + +scenarios(features_path()) diff --git a/tools/openfeature-provider-tck/README.md b/tools/openfeature-provider-tck/README.md index d735a5c5..ca2e40c0 100644 --- a/tools/openfeature-provider-tck/README.md +++ b/tools/openfeature-provider-tck/README.md @@ -112,11 +112,31 @@ definitions never talk to a backend directly, which is why the same Gherkin runs containerised backend and against a provider manipulated in-process. **If your provider talks to a backend, drive it over the HTTP control API** — the document is -available as `control_api_spec()`. That API is the normative contract for those providers, and it is -what makes a conformance claim portable: another language's TCK drives the same endpoints against -the same stack and must get the same answers. +available as `control_api_spec()`, and `HttpControl` is the client for it. That API is the normative +contract for those providers, and it is what makes a conformance claim portable: another language's +TCK drives the same endpoints against the same stack and must get the same answers. -Two of its requirements are easy to get wrong: +```python +control = HttpControl(f"http://localhost:{container.get_launchpad_port()}") +``` + +`HttpControl` is built on `urllib.request` alone, so the TCK gains no HTTP client and no container +dependency. **Orchestrating the stack stays with you**, where the vendor-specific knowledge already +lives — which compose file, which services, which internal ports. That is a deliberate trade against +the "provider authors write no test infrastructure" goal, and worth revisiting once a second +containerised adopter shows what is actually common. + +Two of its behaviours are worth knowing about: + +- **`/reset` is optional and the fallback is automatic.** `prepare_scenario()` prefers `POST /reset`, + which restores the flag baseline with no availability blip; a backend without it answers 404 or + 501 and the client falls back to `POST /start?config=default`. The probe happens once per suite. + flagd-testbed's launchpad registers only `/start`, `/restart`, `/stop` and `/change`, so that + fallback is the normal path today. +- **After a disconnect it starts rather than resets.** `/reset` restores flag *state*; it is not + specified to bring a stopped backend back up. + +Two of the API's requirements are easy to get wrong: - **Containers are never stopped or restarted mid-suite.** Unavailability is simulated *inside* the running stack. Container orchestrators assign host ports dynamically and cannot reliably preserve @@ -175,12 +195,13 @@ subclass rather than a reimplementation, and why it should port back to the SDK | `test_in_memory_conformance` | the SDK's `InMemoryProvider` | reference adoption for a backend-less provider | | `test_controllable_conformance` | `ControllableInMemoryProvider` | the only suite that exercises the configuration-change path — see finding 2 | | `test_in_process_control` | `InProcessControl` | pins what the Gherkin cannot assert about itself | +| `test_http_control` | `HttpControl` | the `/reset` fallback and the disconnect bookkeeping, against a stubbed control API | ``` -56 passed, 7 skipped, 2 xfailed +73 passed, 7 skipped, 2 xfailed ``` -No Docker, no network, under a second. +No Docker and no network beyond loopback. ## Known gaps @@ -190,7 +211,9 @@ No Docker, no network, under a second. `openfeature-flagd-api-testkit` already does for the flagd test harness. - **Evaluation context passthrough is unverifiable.** The scenarios build evaluation contexts but cannot assert one *reached* the backend. That needs an echo operation on the control API. -- **No HTTP control client yet.** It arrives with the first containerised adopter. +- **No shared containerised-backend helper.** `HttpControl` drives the control API, but starting the + stack and discovering its mapped ports is still each adopter's own code. Abstracting that from a + single example tends to produce the wrong abstraction; it should wait for a second adopter. - **Caching, hooks and flag metadata** are not covered. [appendix-a]: https://github.com/open-feature/spec/blob/main/specification/appendix-a-included-utilities.md diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/__init__.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/__init__.py index 31e9d39d..b025cecd 100644 --- a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/__init__.py +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/__init__.py @@ -56,6 +56,11 @@ def tck_config(): ConnectionControl, UnsupportedControlError, ) +from .httpcontrol import ( + DEFAULT_CONFIGURATION, + ControlApiError, + HttpControl, +) from .inprocess import InProcessControl from .provider import ( CHANGING_FLAG_KEY, @@ -66,10 +71,13 @@ def tck_config(): __all__ = [ "ALL_CAPABILITIES", "CHANGING_FLAG_KEY", + "DEFAULT_CONFIGURATION", "BackendControl", "Capability", "ConnectionControl", + "ControlApiError", "ControllableInMemoryProvider", + "HttpControl", "InProcessControl", "TckConfig", "UnsupportedControlError", diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/httpcontrol.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/httpcontrol.py new file mode 100644 index 00000000..ffa265df --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/httpcontrol.py @@ -0,0 +1,262 @@ +"""HTTP backend control: the normative control path for a provider with a real backend.""" + +from __future__ import annotations + +import threading +import typing +import urllib.error +import urllib.parse +import urllib.request + +from .control import BackendControl, ConnectionControl + +__all__ = ["DEFAULT_CONFIGURATION", "ControlApiError", "HttpControl"] + +DEFAULT_CONFIGURATION = "default" +"""The configuration name every backend under test must support. + +It is the one that serves the canonical flag set the feature files assume. +""" + +DEFAULT_TIMEOUT = 30.0 +"""Seconds bounding a single control-API request. + +Control calls are local HTTP to a container on the same host; anything slower +than this is a wedged backend rather than a slow one. +""" + +_NOT_IMPLEMENTED = frozenset({404, 501}) +"""How a backend that does not implement ``/reset`` answers it, per the OpenAPI document.""" + +_SUPPORTED_SCHEMES = frozenset({"http", "https"}) + + +class ControlApiError(RuntimeError): + """Raised when a control-API call fails or answers with an unexpected status. + + Always a defect in the stack under test or in its wiring, never a provider + defect -- so it is raised rather than swallowed. A control call that quietly + did nothing would leave the next scenario running against an unknown backend + state and reporting whatever it found as a conformance result. + """ + + +class HttpControl: + """Drives a backend under test over the HTTP control API in ``control-api.yaml``. + + This is the normative control path for any provider with a real backend, and + it is what makes a conformance claim portable: another language's TCK drives + the same endpoints against the same stack and must get the same answers. + + Built on :mod:`urllib.request` alone, so adopting the TCK pulls in no HTTP + client and no container library. Orchestrating the stack stays with the + adopting suite, where the vendor-specific knowledge already lives -- which + compose file, which services, which internal ports. + + **What it never does.** It never stops, kills or recreates a container. + Unavailability is simulated inside the running stack, through ``POST /stop``, + because container orchestrators assign host ports dynamically and cannot + reliably preserve them across a restart: a restarted backend generally comes + back on a different host port, silently invalidating every provider already + pointed at the old one, and the resulting failure looks like a flaky provider + rather than a broken test. Starting and stopping the stack itself belongs to + the adopting suite, once per session. + + **Scenario isolation.** :meth:`prepare_scenario` prefers ``POST /reset``, + which restores the flag baseline with no availability blip and therefore + cannot inject a spurious lifecycle event into the next scenario. That + operation is optional, and a backend that does not implement it answers 404 + or 501; the TCK then falls back to ``POST /start?config=...``, which also + resets flag state at the cost of a process restart. The fallback is probed + once and remembered for the rest of the suite. + + **After a disconnect, ``/start`` rather than ``/reset``.** ``/reset`` is + specified to restore flag state, not to bring a stopped backend back up, so + a disconnect is recorded and the scenario that follows one is prepared with + ``/start``. + + Safe to share between suites, and it should be shared whenever they drive the + same backend: the disconnect bookkeeping is only correct if every operation + against one backend goes through one instance of this class. + """ + + def __init__( + self, + base_url: str, + *, + configuration: str = DEFAULT_CONFIGURATION, + timeout: float = DEFAULT_TIMEOUT, + ) -> None: + """Build a control for the backend whose control API is rooted at ``base_url``. + + :param base_url: root of the control API, for example + ``http://localhost:32768``. It must be built from the dynamically + mapped host port of the control service, discovered after the stack + is up -- a stack under test must not pin host ports. + :param configuration: the named flag configuration to seed. Defaults to + :data:`DEFAULT_CONFIGURATION`, the only name every backend must + support and the one serving the canonical flag set. + :param timeout: seconds bounding a single control-API request. + """ + parsed = urllib.parse.urlsplit(base_url) + if parsed.scheme not in _SUPPORTED_SCHEMES or not parsed.netloc: + msg = ( + f"base_url {base_url!r} is not an http(s) URL. It is the root of the " + f"control API, built from the dynamically mapped host port of the " + f"control service, for example 'http://localhost:32768'" + ) + raise ValueError(msg) + + self._base_url = base_url.rstrip("/") + self._configuration = configuration + self._timeout = timeout + + self._lock = threading.Lock() + # None until the first /reset call tells us which way it went. + self._reset_supported: bool | None = None + # Set by any operation that may have left the backend down, so the next + # prepare_scenario starts it rather than merely resetting flag state. + self._backend_maybe_down = False + + @property + def description(self) -> str: + return f"the backend at {self._base_url}, driven over the control API" + + def prepare_scenario(self) -> None: + """Bring the backend to the state every scenario starts from. + + Prefers ``/reset`` and falls back to ``/start`` -- see the class + documentation for why, and for why a disconnect forces ``/start``. + """ + with self._lock: + must_start = self._backend_maybe_down or self._reset_supported is False + + if must_start: + self._start() + return + + status = self._call("/reset") + + if status in _NOT_IMPLEMENTED: + # The documented fallback. Remembered so the probe costs one request + # per suite rather than one per scenario. + with self._lock: + self._reset_supported = False + self._start() + return + + if not self._is_success(status): + msg = f"POST /reset on {self._base_url} returned {status}" + raise ControlApiError(msg) + + with self._lock: + self._reset_supported = True + + def change_flag(self) -> None: + """Mutate flag configuration so a conforming provider observes a change.""" + self._require("/change") + + def disconnect(self) -> None: + """Make the backend unreachable, without touching any container. + + The backend *process* inside the still-running container is stopped. See + the class documentation for why that distinction is a requirement rather + than a preference. + """ + with self._lock: + self._backend_maybe_down = True + self._require("/stop") + + def reconnect(self) -> None: + """Make the backend reachable again, preserving flag state. + + Starting with the configuration already in effect restores the same + baseline, so the provider observes a change in availability and never a + change in flag values. + """ + self._start() + + def restart(self, seconds: int) -> None: + """Take the backend down for ``seconds`` and bring it back. + + Part of the control API rather than of :class:`~.control.ConnectionControl`: + no scenario drives a bounded outage today, because + :meth:`disconnect`/:meth:`reconnect` let a scenario end the outage when + it is ready instead of guessing how long a provider needs to notice one. + Exposed because the operation is required of every backend and an + adopting suite may want it for its own tests. + + Unlike ``/stop`` followed by ``/start``, this preserves flag state + across the outage. + """ + with self._lock: + self._backend_maybe_down = True + self._require("/restart", {"seconds": str(seconds)}) + with self._lock: + self._backend_maybe_down = False + + def _start(self) -> None: + self._require("/start", {"config": self._configuration}) + with self._lock: + self._backend_maybe_down = False + + def _require(self, path: str, query: dict[str, str] | None = None) -> None: + """Perform a control call and fail on any non-2xx response.""" + status = self._call(path, query) + if not self._is_success(status): + msg = f"POST {path} on {self._base_url} returned {status}" + raise ControlApiError(msg) + + def _call(self, path: str, query: dict[str, str] | None = None) -> int: + """Perform one control-API request and return its status code. + + The response body is read and discarded: the control API's bodies are + human-readable messages the TCK is specified never to interpret, and + reading them lets the connection be released cleanly. + """ + target = self._base_url + path + if query: + target += "?" + urllib.parse.urlencode(query) + + # An empty body rather than none, so the request carries Content-Length + # even where a proxy in the stack insists on one. + # + # S310 wants the scheme audited before a URL is opened; __init__ rejects + # any base_url that is not http(s), and target is built from that + # validated base URL plus a literal path, so no other scheme can reach + # here. + request = urllib.request.Request(target, data=b"", method="POST") # noqa: S310 + + try: + with urllib.request.urlopen(request, timeout=self._timeout) as response: # noqa: S310 + response.read() + return int(response.status) + except urllib.error.HTTPError as error: + # A status the server chose to report as an error is still an answer, + # and /reset answering 404 is the documented way to say "not + # implemented" -- so this is a return, not a raise. + with error: + error.read() + return int(error.code) + except OSError as error: + msg = ( + f"control request POST {target} failed: {error}. The control API must " + f"stay reachable even while the backend is deliberately down, " + f"otherwise an outage cannot be ended" + ) + raise ControlApiError(msg) from error + + @staticmethod + def _is_success(status: int) -> bool: + return 200 <= status < 300 + + +if typing.TYPE_CHECKING: + # Static assertion, erased at runtime: HttpControl must satisfy both control + # protocols, the way Go's `var _ BackendControl = (*HTTPControl)(nil)` does. + # A method renamed out of the protocol fails type-checking rather than at the + # first scenario that needs it. + def _implements( + control: HttpControl, + ) -> tuple[BackendControl, ConnectionControl]: + return control, control diff --git a/tools/openfeature-provider-tck/tests/test_http_control.py b/tools/openfeature-provider-tck/tests/test_http_control.py new file mode 100644 index 00000000..4c8e80ef --- /dev/null +++ b/tools/openfeature-provider-tck/tests/test_http_control.py @@ -0,0 +1,223 @@ +"""What the Gherkin cannot assert about the HTTP control path. + +Every scenario's isolation rests on :meth:`HttpControl.prepare_scenario` doing +the right thing against a backend that implements only part of the control API, +and on a disconnect being remembered. Both are invisible from inside a scenario: +a control that silently did nothing would leave each scenario running against +whatever state the previous one left behind, and the suite would report those +results as conformance. + +So the control API is stubbed with :mod:`http.server` -- no Docker, no network +beyond loopback -- and the requests it actually made are asserted. +""" + +from __future__ import annotations + +import threading +import typing +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer + +import pytest + +from openfeature.contrib.tools.provider_tck import ( + ControlApiError, + HttpControl, +) + + +class _StubControlApi: + """A control API that records every request and answers a scripted status.""" + + def __init__(self, statuses: dict[str, int] | None = None) -> None: + self.requests: list[tuple[str, str, str]] = [] + """(method, path, query) of every request, in order.""" + + self.statuses = statuses or {} + stub = self + + class Handler(BaseHTTPRequestHandler): + def do_POST(self) -> None: + path, _, query = self.path.partition("?") + stub.requests.append(("POST", path, query)) + status = stub.statuses.get(path, 200) + body = b'{"status":"stub"}' + self.send_response(status) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args: typing.Any) -> None: + """Silence the default stderr logging.""" + + self._server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + self._thread = threading.Thread(target=self._server.serve_forever, daemon=True) + + @property + def base_url(self) -> str: + host, port = self._server.server_address[:2] + return f"http://{host!s}:{port}" + + @property + def paths(self) -> list[str]: + return [path for _, path, _ in self.requests] + + def __enter__(self) -> _StubControlApi: + self._thread.start() + return self + + def __exit__(self, *_exc: object) -> None: + self._server.shutdown() + self._server.server_close() + self._thread.join(timeout=5) + + +@pytest.fixture +def stub() -> typing.Iterator[_StubControlApi]: + with _StubControlApi() as api: + yield api + + +def test_prepare_scenario_prefers_reset_when_the_backend_implements_it( + stub: _StubControlApi, +) -> None: + """The preferred primitive, because it causes no availability blip. + + A ``/start`` between scenarios restarts the backend process, which a + provider observes as an outage and may report as a lifecycle event in the + scenario that follows. + """ + control = HttpControl(stub.base_url) + + control.prepare_scenario() + control.prepare_scenario() + + assert stub.paths == ["/reset", "/reset"] + + +def test_prepare_scenario_falls_back_to_start_and_remembers_the_answer() -> None: + """The path flagd-testbed actually takes: its launchpad has no ``/reset``. + + The fallback must be probed once rather than once per scenario -- a wasted + 404 before every scenario is a slow suite, and hiding the probe entirely + would mean a backend that grows ``/reset`` never gets used properly. + """ + with _StubControlApi({"/reset": 404}) as stub: + control = HttpControl(stub.base_url) + + control.prepare_scenario() + control.prepare_scenario() + control.prepare_scenario() + + assert stub.paths == ["/reset", "/start", "/start", "/start"] + + +@pytest.mark.parametrize("status", [404, 501]) +def test_both_documented_not_implemented_statuses_trigger_the_fallback( + status: int, +) -> None: + """The OpenAPI document permits either, so neither may be treated as a failure.""" + with _StubControlApi({"/reset": status}) as stub: + HttpControl(stub.base_url).prepare_scenario() + + assert stub.paths == ["/reset", "/start"] + + +def test_the_scenario_after_a_disconnect_starts_the_backend( + stub: _StubControlApi, +) -> None: + """``/reset`` restores flag state; it is not specified to start a stopped backend. + + Without this the scenario following a disconnect would prepare a backend + that is still down, register a provider against it, and report the failure + as a provider defect. + """ + control = HttpControl(stub.base_url) + control.prepare_scenario() # settles on /reset, which this stub supports + stub.requests.clear() + + control.disconnect() + control.prepare_scenario() + + assert stub.paths == ["/stop", "/start"] + + +def test_reconnect_starts_the_backend_and_clears_the_disconnect( + stub: _StubControlApi, +) -> None: + """A scenario that ended its own outage leaves the backend up, so ``/reset`` is fine again.""" + control = HttpControl(stub.base_url) + control.prepare_scenario() + control.disconnect() + control.reconnect() + stub.requests.clear() + + control.prepare_scenario() + + assert stub.paths == ["/reset"] + + +def test_start_names_the_configuration_under_test() -> None: + """``default`` is the only name every backend must support, and it serves the canonical set.""" + with _StubControlApi({"/reset": 404}) as stub: + HttpControl(stub.base_url).prepare_scenario() + + assert ("POST", "/start", "config=default") in stub.requests + + +def test_a_custom_configuration_is_carried_through() -> None: + with _StubControlApi({"/reset": 404}) as stub: + HttpControl(stub.base_url, configuration="ssl").prepare_scenario() + + assert ("POST", "/start", "config=ssl") in stub.requests + + +def test_restart_carries_the_outage_duration(stub: _StubControlApi) -> None: + HttpControl(stub.base_url).restart(7) + + assert ("POST", "/restart", "seconds=7") in stub.requests + + +def test_change_flag_posts_to_change(stub: _StubControlApi) -> None: + HttpControl(stub.base_url).change_flag() + + assert stub.paths == ["/change"] + + +def test_a_failed_control_call_raises_rather_than_passing_silently() -> None: + """A control call that did nothing would leave the next scenario in an unknown state.""" + with ( + _StubControlApi({"/change": 500}) as stub, + pytest.raises(ControlApiError, match="500"), + ): + HttpControl(stub.base_url).change_flag() + + +def test_an_unreachable_control_api_raises_with_the_reason() -> None: + """The control API must stay up even while the backend is deliberately down.""" + # Bound and immediately closed, so the port is almost certainly free. + with _StubControlApi() as stub: + base_url = stub.base_url + control = HttpControl(base_url, timeout=2.0) + + with pytest.raises(ControlApiError, match="control request POST"): + control.change_flag() + + +@pytest.mark.parametrize( + "base_url", + ["", "localhost:8080", "file:///etc/passwd", "ftp://localhost:8080"], +) +def test_a_base_url_that_is_not_an_http_url_is_rejected_at_construction( + base_url: str, +) -> None: + """Rejected early, and by scheme, so no other URL scheme can reach ``urlopen``.""" + with pytest.raises(ValueError, match="not an http"): + HttpControl(base_url) + + +def test_a_trailing_slash_does_not_produce_a_double_slash_path() -> None: + with _StubControlApi() as stub: + HttpControl(stub.base_url + "/").change_flag() + + assert stub.paths == ["/change"] diff --git a/uv.lock b/uv.lock index b0ab379c..d23aa347 100644 --- a/uv.lock +++ b/uv.lock @@ -1886,6 +1886,7 @@ dev = [ { name = "coverage", extra = ["toml"] }, { name = "grpcio-health-checking" }, { name = "mypy" }, + { name = "openfeature-provider-tck" }, { 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-provider-tck", editable = "tools/openfeature-provider-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" },