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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions src/quant_platform_kit/common/health.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,26 @@ def read(self) -> Heartbeat | None:

def _register_flask(self) -> None:
from flask import jsonify
monitor = self

@self._app.route("/health", methods=["GET"])
@self._app.route("/healthz", methods=["GET"])
def health():
def qpk_health():
return jsonify({"status": "ok", "timestamp": datetime.now(timezone.utc).isoformat()})

existing_rules = {getattr(rule, "rule", "") for rule in self._app.url_map.iter_rules()}
if "/health" not in existing_rules:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Check methods before skipping health routes

This path-only check suppresses the GET health handler whenever any /health rule already exists, even if that rule only handles another method such as POST; in that app register_health_endpoint(app) still promises to add GET /health, but GET requests will return 405 because Flask allows separate rules for the same path with different methods. The same issue applies to the /healthz check below, so skip only when an existing rule actually handles GET/HEAD.

Useful? React with 👍 / 👎.

self._app.add_url_rule(
"/health",
endpoint="qpk_health",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid fixed endpoint names for health routes

Using a fixed qpk_health endpoint still allows the same collision this change is trying to harden against: if the host Flask app already has any view registered with endpoint/function name qpk_health, or defines one after calling register_health_endpoint, Flask raises an endpoint-overwrite assertion even though /health itself may be free. Generate or probe for an unused endpoint name before registering the QPK health view.

Useful? React with 👍 / 👎.

view_func=qpk_health,
methods=["GET"],
)
if "/healthz" not in existing_rules:
self._app.add_url_rule(
"/healthz",
endpoint="qpk_healthz",
view_func=qpk_health,
methods=["GET"],
)

def _start_http_server(self) -> None:
import http.server
port = self._http_port
Expand Down
71 changes: 71 additions & 0 deletions tests/test_common_health.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
from __future__ import annotations

import sys
import types
from collections.abc import Callable
from dataclasses import dataclass
from typing import Any

from quant_platform_kit.common.health import register_health_endpoint


@dataclass
class _FakeRule:
rule: str


class _FakeUrlMap:
def __init__(self, app: _FakeFlaskApp):
self._app = app

def iter_rules(self):
return iter(self._app.rules)


class _FakeFlaskApp:
def __init__(self):
self.rules: list[_FakeRule] = []
self.view_functions: dict[str, Callable[..., Any]] = {}
self.url_map = _FakeUrlMap(self)

def add_url_rule(
self,
rule: str,
endpoint: str,
view_func: Callable[..., Any],
methods: list[str],
) -> None:
if endpoint in self.view_functions:
raise AssertionError(
f"View function mapping is overwriting an existing endpoint function: {endpoint}"
)
self.rules.append(_FakeRule(rule))
self.view_functions[endpoint] = view_func


def _install_fake_flask(monkeypatch):
monkeypatch.setitem(sys.modules, "flask", types.SimpleNamespace(jsonify=lambda payload: payload))


def test_register_health_endpoint_uses_non_colliding_endpoint_names(monkeypatch):
_install_fake_flask(monkeypatch)
app = _FakeFlaskApp()
register_health_endpoint(app)

app.add_url_rule("/", endpoint="health", view_func=lambda: "service-info", methods=["GET"])

assert {rule.rule for rule in app.rules} == {"/health", "/healthz", "/"}
assert {"qpk_health", "qpk_healthz", "health"} <= set(app.view_functions)


def test_register_health_endpoint_preserves_existing_health_route(monkeypatch):
_install_fake_flask(monkeypatch)
app = _FakeFlaskApp()
app.add_url_rule("/health", endpoint="platform_health", view_func=lambda: "platform-ok", methods=["GET"])

register_health_endpoint(app)

assert [rule.rule for rule in app.rules] == ["/health", "/healthz"]
assert "platform_health" in app.view_functions
assert "qpk_health" not in app.view_functions
assert "qpk_healthz" in app.view_functions
Loading