mirror of
https://github.com/taylanbakircioglu/haproxy-openmanager.git
synced 2026-09-12 05:48:58 +00:00
fix(vip): scope the HA/VIP page to the selected cluster (v1.10.7)
The page ignored the cluster picker in the header. Both the VIP table and the v1.10.4 "Unmanaged keepalived detected" panel queried the whole fleet, so on an install with more than one cluster the lists showed every cluster's nodes at once and did not change when the selection did - the panel looked stuck on one cluster's keepalived. - GET /api/vip/discoveries takes the same optional cluster_id the VIP list already took, mapped to the cluster's pool via haproxy_clusters exactly like list_vips does. Omitting it still returns the whole fleet, so no existing caller changes behaviour. - VIPManagement reads selectedCluster from ClusterContext (it only took the cluster list before) and sends cluster_id on both fetches. The fetch callbacks depend on the scope, so switching cluster refetches instead of showing stale rows. - tests/test_vip_discoveries_scope.py pins both defects this endpoint has had: that the route reaches its own handler rather than being parsed as a vip_id (asserting != 422 specifically, since the repo's generic endpoint-auth tests accept 422 alongside 401/403 and therefore could not catch it), and that the cluster filter resolves cluster -> pool and short-circuits when absent. Behaviour change worth calling out: the VIP table is now scoped to the selected cluster where it was fleet-wide before. The API still serves the fleet-wide view to any caller that omits cluster_id. Backend suite: 1345 passed, 152 skipped. Frontend production build clean.
This commit is contained in:
taylanbakircioglu
@@ -2474,7 +2474,8 @@ Developed with ❤️ for the HAProxy community
|
||||
|
||||
## Release Notes
|
||||
|
||||
- **v1.10.6** (2026-08-13) — **VIP adoption panel was unreachable**: v1.10.4's *Unmanaged keepalived detected* panel never appeared, even on a fleet where the agents had reported their configs correctly. `GET /discoveries` was declared **after** `GET /{vip_id}` in `routers/vip.py`, and FastAPI matches routes in declaration order, so every request for the discovery list was answered by the get-one-VIP handler, which takes `vip_id: int` and rejected `"discoveries"` with **422** before the real handler ran. Nothing surfaced the failure: the agents reported normally, the rows landed in `vip_discoveries`, and the HA/VIP page treats any non-OK response as "nothing to show" — so the whole feature was invisible with no error anywhere. The route is moved above the parameterised ones, and a static source scan now asserts that **no** literal path in **any** router is shadowed by an earlier parameterised one, so the class of bug cannot come back silently. Data reported under v1.10.4 is not lost: existing `vip_discoveries` rows appear as soon as the fixed backend is deployed, with no agent action needed. Backend-only fix. No schema, API-shape or agent change. On the public repo this is the first artifact carrying v1.10.4 and v1.10.5: neither was released separately, because v1.10.4's adoption panel could not work until this fix landed.
|
||||
- **v1.10.7** (2026-08-13) — **HA / VIP follows the selected cluster**: the page ignored the cluster picker in the header. On a multi-cluster install both the VIP table and the new *Unmanaged keepalived detected* panel listed every cluster's nodes at once and did not change when the selection did, so the panel appeared to be stuck on one cluster's keepalived. Both lists now send `cluster_id`, resolved to that cluster's pool exactly as the Apply Management view already did. The API parameter is **optional**: a caller that omits it still receives the whole fleet, so nothing outside the page changes. This is a deliberate behaviour change for the VIP table, which was fleet-wide before. Backend and frontend only: no schema, no agent change. On the public repo this is the first artifact carrying v1.10.4, v1.10.5 and v1.10.6: none was released separately, because the adoption panel could not work until these fixes landed.
|
||||
- **v1.10.6** (2026-08-13) — **VIP adoption panel was unreachable**: v1.10.4's *Unmanaged keepalived detected* panel never appeared, even on a fleet where the agents had reported their configs correctly. `GET /discoveries` was declared **after** `GET /{vip_id}` in `routers/vip.py`, and FastAPI matches routes in declaration order, so every request for the discovery list was answered by the get-one-VIP handler, which takes `vip_id: int` and rejected `"discoveries"` with **422** before the real handler ran. Nothing surfaced the failure: the agents reported normally, the rows landed in `vip_discoveries`, and the HA/VIP page treats any non-OK response as "nothing to show" — so the whole feature was invisible with no error anywhere. The route is moved above the parameterised ones, and a static source scan now asserts that **no** literal path in **any** router is shadowed by an earlier parameterised one, so the class of bug cannot come back silently. Data reported under v1.10.4 is not lost: existing `vip_discoveries` rows appear as soon as the fixed backend is deployed, with no agent action needed. Backend-only fix. No schema, API-shape or agent change.
|
||||
- **v1.10.5** (2026-08-09) — **HTTP-01 challenge backend on split deployments**: on a deployment where the HAProxy nodes and the management stack are on different hosts, HTTP-01 issuance could fail silently for weeks while DNS-01 kept working — the rendered config pointed `server _acme_mgmt` at an address that resolves **on the HAProxy node**, defaulting to loopback, and every diagnostic still reported success. The per-cluster `acme_backend_url` now has a UI field, changing it actually mints a config version, and the value is validated where it is written. Three adjacent bugs are fixed with it: a config-generation failure was returned as `# Error ...` text and then stored as an APPLIED version and pushed to agents as the cluster's whole `haproxy.cfg` (both call sites now refuse with 422); a nullable `frontends.mode` was interpolated raw and emitted `mode None`, which HAProxy rejects and which takes down the entire cluster config; and cluster creation silently dropped the ACME fields. `docker-compose.yml` now interpolates `PUBLIC_URL` / `MANAGEMENT_BASE_URL` instead of hardcoding them, with the old literals as defaults. Diagnostics read the response body so an SPA answering 200 is no longer counted as healthy, and every new condition is a warning rather than a failure so no install is locked on upgrade. No schema, API-shape or agent change.
|
||||
- **v1.10.4** (2026-08-08) — **Adopt an existing keepalived VIP** (Issue #27 follow-up): on a fleet that already runs keepalived, the **HA / VIP** page came up empty, because the flow was one-way — VIPs were declared in OpenManager and pushed to the node, and nothing ever read what was already there. Agents now **report the `keepalived.conf` they find and do not own** (strictly read-only; the node is never touched), the page lists those nodes under *Unmanaged keepalived detected*, and **Adopt** turns one `vrrp_instance` into a managed VIP with the values from the file instead of retyping them. The heartbeat could not drive this: it carries the VIP address and a best-effort MASTER/BACKUP, while rendering a node's config needs **eleven** fields, and guessing them is not cosmetic — a wrong `virtual_router_id` puts the nodes in separate VRRP domains and a wrong `auth_pass` makes them reject each other, so both would claim the VIP. Because adoption **replaces** the operator's file with OpenManager's render, the parser reports every directive it cannot reproduce — a `notify_master` hook, an LVS `virtual_server` section, a `vrrp_sync_group`, a second address in one instance, a custom `track_script` — and **refuses** while any remain; the operator can waive that class explicitly, but a value that is simply *unknown* (an absent VRID or prefix length) can never be waived, only supplied. keepalived's own documented defaults (`state BACKUP`, `priority 100`, `advert_int 1`) are applied and shown as assumed. The agent's ownership guard is **not** weakened: adoption authorises exactly **one** takeover of exactly the file that was analysed, pinned to its hash, so a config edited between adoption and Apply is still refused. The adopted VIP is created **PENDING** like any other, so nothing reaches the node until it is applied from Apply Management. VRRP passwords are Fernet-encrypted at ingest and masked in the stored copy and the preview. Schema change: one new table `vip_discoveries` plus two additive columns (SCHEMA_VERSION 10 → 11, auto-migrated, no existing table altered) — **see the upgrade notes: this bump re-seeds the four built-in roles, and the Linux agent script must reach the nodes before discovery starts**.
|
||||
- **v1.10.3** (2026-08-08) — **Multi-account ACME: the certificate wizard honours the account you pick**: with more than one ACME account registered, picking an **HTTP-01** account in *Request ACME Certificate* still produced a **DNS-01** request. Three faults compounded. (1) `Form.useWatch` reports only fields that are currently **rendered**, and the account `Select` lives on the *Configuration* step — so as soon as the wizard advanced to *Review* the watch read `undefined` and the wizard silently reverted to the default account, even though the value was still in the form store; the watches now pass `preserve: true`. The same fault disabled the **wildcard guard** on *Review*, the one step where Submit lives. (2) The UI and the backend disagreed on which account is the *default*: the backend takes the **newest** valid account (`ORDER BY created_at DESC`), the UI took the **oldest** entry of a list ordered by id — the opposite account whenever the two differ. The wizard now resolves the same one, and sends `account_id` **explicitly** so there is no guess left to disagree about. (3) `account_id` was read from the form store while `challenge_type` came from the reverted account object, so the request asked for DNS-01 validation on an HTTP-01 account and the API answered `The selected ACME account has no DNS provider configured for DNS-01.` — both are now derived from one resolved account. The *Review* step also showed the default account's address instead of the chosen one, and Submit stayed enabled for a deactivated account; both fixed. Frontend only — no schema, API-shape, agent or rendered-config changes, and single-account installations behave exactly as before.
|
||||
|
||||
@@ -1,3 +1,25 @@
|
||||
# Upgrade Notes — v1.10.7 (HA / VIP follows the selected cluster)
|
||||
|
||||
**Backend + frontend, no schema change.** No `SCHEMA_VERSION` bump, so the built-in roles are
|
||||
**not** re-seeded. No agent impact.
|
||||
|
||||
- **The HA / VIP page ignored the cluster picker.** Both the VIP table and the *Unmanaged
|
||||
keepalived detected* panel queried the whole fleet, so on an install with more than one
|
||||
cluster the lists never changed when the selection did. Both now pass `cluster_id`, mapped to
|
||||
the cluster's pool the same way `GET /api/vip?cluster_id=` already worked for Apply
|
||||
Management.
|
||||
- **Behaviour change worth knowing:** the VIP table is now scoped to the selected cluster. It
|
||||
used to show every VIP in the fleet. If you relied on the fleet-wide view, the API still
|
||||
supports it — `GET /api/vip` and `GET /api/vip/discoveries` without `cluster_id` return
|
||||
everything, unchanged.
|
||||
- **API compatibility:** `cluster_id` is optional on both endpoints. Existing integrations that
|
||||
do not send it behave exactly as before.
|
||||
|
||||
**Rollback:** safe. The change is a query parameter plus the page that sends it; reverting
|
||||
restores the fleet-wide lists and touches no data.
|
||||
|
||||
---
|
||||
|
||||
# Upgrade Notes — v1.10.6 (VIP adoption panel was unreachable)
|
||||
|
||||
**One backend fix, no schema change.** No `SCHEMA_VERSION` bump, so the built-in roles are
|
||||
|
||||
@@ -465,11 +465,15 @@ async def list_vips(cluster_id: Optional[int] = None, authorization: str = Heade
|
||||
# non-OK response as "nothing to show", so the whole adoption feature silently disappears.
|
||||
# See the v1.10.4 adoption section further down for the endpoint's own documentation.
|
||||
@router.get("/discoveries")
|
||||
async def list_vip_discoveries(authorization: str = Header(None)):
|
||||
async def list_vip_discoveries(cluster_id: Optional[int] = None, authorization: str = Header(None)):
|
||||
"""Unmanaged keepalived configs the agents found on their nodes.
|
||||
|
||||
Read-only and safe to poll: this is what the HA/VIP page shows so an existing VIP is
|
||||
visible before anyone adopts it.
|
||||
|
||||
`cluster_id` scopes the result to the agents in that cluster's pool, resolved exactly like
|
||||
the VIP list above. Without it every discovery in the fleet is returned, which is what a
|
||||
caller that does not know about the parameter still gets.
|
||||
"""
|
||||
await _require(authorization, "read")
|
||||
conn = await get_database_connection()
|
||||
@@ -480,8 +484,10 @@ async def list_vip_discoveries(authorization: str = Header(None)):
|
||||
FROM vip_discoveries d
|
||||
JOIN agents a ON a.id = d.agent_id
|
||||
LEFT JOIN haproxy_cluster_pools p ON p.id = a.pool_id
|
||||
WHERE $1::int IS NULL
|
||||
OR a.pool_id = (SELECT pool_id FROM haproxy_clusters WHERE id = $1::int)
|
||||
ORDER BY d.reported_at DESC, d.id DESC
|
||||
""")
|
||||
""", cluster_id)
|
||||
except Exception as exc: # noqa: BLE001 — a missing relation degrades to empty (B-7)
|
||||
logger.debug(f"vip_discoveries unavailable: {exc}")
|
||||
return {"discoveries": []}
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
"""
|
||||
v1.10.6 — GET /api/vip/discoveries resolves to its own handler and honours cluster_id.
|
||||
|
||||
Two defects, one endpoint, found in that order on a live fleet:
|
||||
|
||||
1. The route was declared after `GET /{vip_id}`, so FastAPI matched it there and answered
|
||||
422 ("discoveries" is not an int) before the handler ran. The static declaration-order
|
||||
guard lives in test_router_path_shadowing.py; this file pins the observable behaviour,
|
||||
because a 422 is what the browser actually saw.
|
||||
|
||||
2. Once reachable, it returned every discovery in the fleet regardless of the cluster
|
||||
selected in the header, so a multi-cluster install saw one undifferentiated list. The
|
||||
endpoint now takes the same optional `cluster_id` the VIP list takes.
|
||||
|
||||
The auth tests here deliberately assert `!= 422`: the repo's generic endpoint-auth tests
|
||||
accept 401/403/422 together, which is precisely why defect 1 slipped through them.
|
||||
"""
|
||||
import pathlib
|
||||
import re
|
||||
|
||||
import pytest
|
||||
|
||||
VIP_ROUTER = pathlib.Path(__file__).resolve().parents[1] / "routers" / "vip.py"
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------------
|
||||
# 1. The route reaches its own handler (defect 1)
|
||||
# ----------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.parametrize("path", [
|
||||
"/api/vip/discoveries",
|
||||
"/api/vip/discoveries?cluster_id=7",
|
||||
])
|
||||
def test_discoveries_route_is_not_captured_by_the_vip_id_route(client, path):
|
||||
res = client.get(path)
|
||||
assert res.status_code != 422, (
|
||||
f"GET {path} returned 422 — the request was routed into the get-one-VIP handler, "
|
||||
f"which parses the path segment as an int. Declaration order regressed. "
|
||||
f"Body: {res.text[:200]}"
|
||||
)
|
||||
assert res.status_code in (401, 403), (
|
||||
f"GET {path} without a token should be refused by the vip.read gate, got "
|
||||
f"{res.status_code}. Body: {res.text[:200]}"
|
||||
)
|
||||
|
||||
|
||||
def test_get_one_vip_still_parses_a_numeric_id(client):
|
||||
"""Moving /discoveries above /{vip_id} must not shadow the numeric route itself."""
|
||||
res = client.get("/api/vip/12")
|
||||
assert res.status_code in (401, 403), res.text[:200]
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------------
|
||||
# 2. cluster_id is accepted and actually scopes the query (defect 2)
|
||||
# ----------------------------------------------------------------------------
|
||||
|
||||
def test_handler_accepts_cluster_id():
|
||||
from routers.vip import list_vip_discoveries
|
||||
import inspect
|
||||
|
||||
params = inspect.signature(list_vip_discoveries).parameters
|
||||
assert "cluster_id" in params, (
|
||||
"list_vip_discoveries no longer takes cluster_id; the HA/VIP page would show every "
|
||||
"cluster's nodes at once again"
|
||||
)
|
||||
assert params["cluster_id"].default is None, (
|
||||
"cluster_id must stay optional — omitting it returns the whole fleet, which is what "
|
||||
"a caller that predates the parameter expects"
|
||||
)
|
||||
|
||||
|
||||
def test_discovery_query_scopes_by_the_cluster_pool():
|
||||
"""The filter must resolve cluster -> pool the same way the VIP list does, and must be a
|
||||
no-op when the parameter is absent."""
|
||||
source = VIP_ROUTER.read_text()
|
||||
start = source.index("async def list_vip_discoveries")
|
||||
end = source.index("def _find_candidate", start)
|
||||
body = source[start:end]
|
||||
|
||||
assert "FROM vip_discoveries" in body, "the discovery query moved; re-point this test"
|
||||
assert re.search(r"a\.pool_id\s*=\s*\(\s*SELECT\s+pool_id\s+FROM\s+haproxy_clusters", body), (
|
||||
"the cluster filter must map cluster -> pool via haproxy_clusters, matching list_vips"
|
||||
)
|
||||
assert "IS NULL" in body, (
|
||||
"the filter must short-circuit when cluster_id is absent, so an unscoped call still "
|
||||
"returns the whole fleet"
|
||||
)
|
||||
@@ -1,5 +1,5 @@
|
||||
{
|
||||
"version": "1.10.6",
|
||||
"releaseName": "VIP adoption panel was unreachable (route shadowing)",
|
||||
"version": "1.10.7",
|
||||
"releaseName": "HA/VIP follows the selected cluster",
|
||||
"releaseDate": "2026-08-13"
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "haproxy-openmanager-frontend",
|
||||
"version": "1.10.6",
|
||||
"version": "1.10.7",
|
||||
"description": "HAProxy Load Balancer Management UI",
|
||||
"license": "AGPL-3.0-or-later",
|
||||
"dependencies": {
|
||||
|
||||
@@ -72,7 +72,7 @@ const fetchApiError = async (res, fallback) => {
|
||||
};
|
||||
|
||||
const VIPManagement = () => {
|
||||
const { clusters } = useCluster();
|
||||
const { clusters, selectedCluster } = useCluster();
|
||||
const [vips, setVips] = useState([]);
|
||||
const [loading, setLoading] = useState(false);
|
||||
const [modalVisible, setModalVisible] = useState(false);
|
||||
@@ -103,10 +103,15 @@ const VIPManagement = () => {
|
||||
return Array.from(seen, ([id, name]) => ({ id, name }));
|
||||
}, [clusters]);
|
||||
|
||||
// v1.10.6 — both lists follow the cluster picked in the header, like every other page. The
|
||||
// selector was always there but this page ignored it, so a fleet with several clusters saw
|
||||
// one undifferentiated list. Falls back to fleet-wide while the context is still resolving.
|
||||
const scopeQuery = selectedCluster?.id ? `?cluster_id=${selectedCluster.id}` : '';
|
||||
|
||||
const fetchVips = useCallback(async () => {
|
||||
setLoading(true);
|
||||
try {
|
||||
const res = await fetch('/api/vip', { headers: authHeaders() });
|
||||
const res = await fetch(`/api/vip${scopeQuery}`, { headers: authHeaders() });
|
||||
if (res.ok) {
|
||||
const data = await res.json();
|
||||
setVips(data.vips || []);
|
||||
@@ -119,21 +124,21 @@ const VIPManagement = () => {
|
||||
} finally {
|
||||
setLoading(false);
|
||||
}
|
||||
}, []);
|
||||
}, [scopeQuery]);
|
||||
|
||||
// v1.10.4 — keepalived configs the agents found on their nodes but do NOT manage. This is why
|
||||
// the page could be empty on a fleet that already runs keepalived: the flow was one-way, so
|
||||
// nothing ever read what was already there.
|
||||
const fetchDiscoveries = useCallback(async () => {
|
||||
try {
|
||||
const res = await fetch('/api/vip/discoveries', { headers: authHeaders() });
|
||||
const res = await fetch(`/api/vip/discoveries${scopeQuery}`, { headers: authHeaders() });
|
||||
if (!res.ok) { setDiscoveries([]); return; }
|
||||
const data = await res.json();
|
||||
setDiscoveries((data.discoveries || []).filter((d) => !d.is_managed && !d.adopted_vip_id));
|
||||
} catch (e) {
|
||||
console.error('fetchDiscoveries failed', e);
|
||||
}
|
||||
}, []);
|
||||
}, [scopeQuery]);
|
||||
|
||||
useEffect(() => {
|
||||
fetchVips();
|
||||
|
||||
Reference in New Issue
Block a user