From 2267f24427914618c4b7f3d228908914a728cc72 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 24 Aug 2026 13:56:59 +0200 Subject: [PATCH 1/3] feat(provider-tck): add an HTTP client for the backend control API The normative control path for any provider with a real backend. HttpControl drives the endpoints in control-api.yaml -- /start, /stop, /restart, /change and the optional /reset -- so a containerised provider can adopt the suite without writing its own control client, and so another language's TCK drives the same endpoints against the same stack and must get the same answers. Built on urllib.request alone. The TCK gains no HTTP client and no container dependency: orchestrating the stack stays with the adopting suite, where the vendor-specific knowledge already lives. 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 -- abstracting from one example tends to produce the wrong abstraction. Two behaviours carry the isolation guarantee: * prepare_scenario prefers POST /reset, which restores the flag baseline with no availability blip and so cannot inject a spurious lifecycle event into the next scenario. It is optional; a backend without it answers 404 or 501 and the client falls back to POST /start?config=default. The probe happens once per suite and is remembered. flagd-testbed's launchpad registers only /start, /restart, /stop and /change, so the fallback is the normal path. * After a disconnect the backend may be down, and /reset is specified to restore flag state rather than to start a stopped backend, so a disconnect is recorded and the scenario following one is prepared with /start. Both are invisible from inside a scenario -- a control that silently did nothing would leave each scenario running against whatever the previous one left behind, and the suite would report those results as conformance. So they are pinned by a self-test against a stubbed control API built on http.server: no Docker, no network beyond loopback. Signed-off-by: Simon Schrottner --- tools/openfeature-provider-tck/README.md | 37 ++- .../contrib/tools/provider_tck/__init__.py | 8 + .../contrib/tools/provider_tck/httpcontrol.py | 262 ++++++++++++++++++ .../tests/test_http_control.py | 223 +++++++++++++++ 4 files changed, 523 insertions(+), 7 deletions(-) create mode 100644 tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/httpcontrol.py create mode 100644 tools/openfeature-provider-tck/tests/test_http_control.py 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"] From 7336ad1dba02f445c970e2cf340530487fc90387 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 24 Aug 2026 13:57:14 +0200 Subject: [PATCH 2/3] feat(flagd): run the provider conformance suite against both resolvers Adopts the OpenFeature provider conformance suite in the flagd provider, for both resolvers, as two separate suites. They are separate because they are separately conformant. flagd resolves flags two quite different ways -- RPC evaluates remotely over gRPC, in-process syncs the ruleset and evaluates locally -- and 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 suite exists to surface. 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. 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, and a restarted backend on a new port looks like a flaky provider rather than a broken test. One stack and one HttpControl serve both suites. One flagd process serves both resolver ports, so there is nothing a second stack would isolate -- and the control has to be shared, because it tracks whether a disconnect has left the backend down so the next scenario starts it rather than merely resetting flag state. Two instances would each hold half of that knowledge. Every capability is declared on the strength of a line of provider code rather than of a green run, and each declaration carries its file and line. Both resolvers get the same set: EVENTS, STALE, CONFIGURATION_CHANGE, OBJECT, UNAVAILABLE_INIT and STRICT_NUMERIC_TYPING. TARGETING and CACHING are withheld from both, because no scenario carries either tag and a capability nothing exercises would be a claim with no evidence behind it. Worth recording that STALE is declared for RPC. The Go provider's RPC resolver never emits PROVIDER_STALE -- it sends ProviderError directly on connection loss (go-sdk-contrib#939) -- and the Go adoption withholds the capability for that reason. Python has no such asymmetry: both resolvers emit PROVIDER_STALE from the same channel-connectivity callback shape on TRANSIENT_FAILURE, and only escalate to PROVIDER_ERROR once the retry grace period expires. That is the behaviour the specification describes, and it is the reason the grace period is set well above the length of the scenario's outage: too short a value turns a scenario about staleness into one about failure. Signed-off-by: Simon Schrottner --- .../openfeature-provider-flagd/pyproject.toml | 5 + .../tests/tck/__init__.py | 0 .../tests/tck/conftest.py | 73 ++++++++ .../tests/tck/suite.py | 167 ++++++++++++++++++ .../tests/tck/test_in_process_conformance.py | 96 ++++++++++ .../tests/tck/test_rpc_conformance.py | 97 ++++++++++ 6 files changed, 438 insertions(+) create mode 100644 providers/openfeature-provider-flagd/tests/tck/__init__.py create mode 100644 providers/openfeature-provider-flagd/tests/tck/conftest.py create mode 100644 providers/openfeature-provider-flagd/tests/tck/suite.py create mode 100644 providers/openfeature-provider-flagd/tests/tck/test_in_process_conformance.py create mode 100644 providers/openfeature-provider-flagd/tests/tck/test_rpc_conformance.py 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()) From 2924abdeeb4d0acc6016d77be24e2b6435af70eb Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 24 Aug 2026 14:06:46 +0200 Subject: [PATCH 3/3] fix(flagd): add the provider conformance suite to uv.lock The flagd dev group now depends on openfeature-provider-tck as a workspace source, so the lock has to carry it or uv sync --frozen fails for every package in the workspace. Signed-off-by: Simon Schrottner --- uv.lock | 2 ++ 1 file changed, 2 insertions(+) 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" },