diff --git a/README.md b/README.md index 1dd70a3..f6c42c4 100644 --- a/README.md +++ b/README.md @@ -2475,6 +2475,7 @@ Developed with ❤️ for the HAProxy community ## Release Notes +- **v1.11.1** (2026-08-15) — **A node can no longer be hidden from adoption for good, and Config Import reaches a freshly installed agent**: the agent posts an unmanaged `keepalived.conf` for adoption and caches the hash of what it sent, so the file (which carries the VRRP password) is re-posted only when it changes. Delivery was judged by `curl`'s exit code, which is **0 for 5xx as well**, so a report the server *rejected* was recorded as delivered — and because a hand-maintained config does not change on its own, that node dropped out of *Unmanaged keepalived detected* permanently, curable only by deleting a cache file on the node by hand. Now: the report is cached only on a 2xx; `GET /agents/{name}/keepalived-config` tells the agent whether the server actually holds a discovery for it, so nodes stuck from earlier releases recover by themselves on the next poll; a **400/413/422** records the refusal so identical bytes are not re-posted forever (4xx and 5xx are never sampled out of the request log, so an unattended loop would write a row carrying the whole config every cycle), while **401 and 404 keep retrying** because in this system they mean a token rotation or an agent row briefly absent, not a bad payload. The same exit-code mistake in the *clear* path is fixed too, where it left a managed node still being offered for adoption. Separately, **Config Import silently did nothing on any agent that had never self-upgraded**: `check_config_requests` was defined in the installer and in the self-upgrade daemon but not in the body a fresh install writes, and its call site is guarded by `type`, so the operator asked a node for its `haproxy.cfg` and nothing arrived, with no error anywhere. And the cluster whose `keepalived_config_path` is handed to an agent is now resolved deterministically — a pool may hold several clusters, and the unordered join could return a different one between polls, pointing the agent at a file that does not exist. Agent-script change: sync the script from Agent Management and let the agents upgrade. No schema change. - **v1.11.0** (2026-08-14) — **Unified request/response log with configurable retention**: until now the only record of what happened was `user_activity_logs`, which stores non-GET **2xx** operations with no bodies — so when something failed you could see *that* the count went up, never *what was sent or what came back*. This release adds one queryable timeline covering **both directions**: every inbound API call (**including GETs and including 4xx/5xx**) with the user, client IP, status, duration and — redacted and size-capped — the request and response bodies; and every **outbound** HTTP call the backend makes, tagged with who it went to (ACME/Let's Encrypt, Cloudflare, GoDaddy, HAProxy stats, agents, the ACME diagnostics probe). Outbound rows **inherit the inbound request's id**, so one operator action and the CA/DNS calls it triggered read as a single trace — opening a failed *Request Certificate* shows the exact `POST /acme/new-order` and the CA's `429` body underneath it. Capture is a **pure-ASGI middleware that tees** the request and response streams rather than draining them, so no downstream handler is affected (notably the raw-body agent heartbeat), and rows are written by a **batching background writer** with a bounded queue, so the request path never waits on the database and a saturated logger drops rows visibly instead of blocking. Secrets never land: headers are an allowlist (`Authorization`/`Cookie` reduced to a presence marker), body keys and value shapes are redacted (passwords, tokens, API keys, private-key PEMs, JWTs), the **ACME JWS request body is never stored** (a stored `protected`+`signature` pair is a replayable credential — a summary is logged instead), DNS-provider errors record only the exception **type**, and the ACME HTTP-01 challenge endpoint is excluded so `key_authorization` is never captured. **Retention is operator-configurable** in *Settings → Request Log*: separate day counts for successful and failed rows (defaults 7 and 30) plus a hard row cap (500 000), whichever is reached first, pruned in **batches** under a Postgres advisory lock so a multi-million-row table cannot time out the delete or have every replica scan it at once. New **Request Log** page (`requestlog.read`) and retention/purge permission (`requestlog.manage`); `super_admin` and `security_admin` get both, `operator` gets read, `viewer` gets neither. **Successful agent polls are not logged** (`capture_agent_success`, default off; failures always are), which is what keeps the table's size a function of operator activity rather than of node count: measured at 2 424 bytes/row, a 200-node fleet would otherwise write 2.0M rows/day and reach the row cap in six hours, silently reducing the configured 7-day/30-day retention to a few hours for everything in the table. Cost is measured, not estimated: 27.7 µs per request on the hot path, 18.8 µs per row on the writer task, **0.096 % of one core at 500 nodes**. Adds one new table (`request_logs`) and its settings seed — SCHEMA_VERSION 11 → 12 (not 11: that number was taken by v1.10.4 while this was in review, and the version gate would have skipped the migration entirely on every existing install), auto-migrated, no existing table altered, no agent or rendered-config change. Kill switches: `REQUEST_LOG_ENABLED=false` (environment — the middleware is then never registered and costs nothing) or the `enabled` toggle in Settings (no restart). - **v1.10.14** (2026-08-14) — **A converged node keeps acknowledging**: the deploy report is the server's only evidence that a member node applied its `keepalived.conf`, and it was sent on the write path alone. Once the rendered config was on disk the agent took the idempotency early return on every cycle and never reported again, so a **single lost report** — a backend restart, a 5xx, a network blip — left the VIP reading `SYNCING (0/n)` with an empty *Last ack* forever, while the node was demonstrably running the right config. Nothing would ever reconcile the two: the node was correct, the page was not, and the only way out was to change the rendered config so the agent wrote it again. The agent now re-asserts its state on the idempotent path too, which costs one request per node per ~2.5 minutes and touches nothing on the node — keepalived is not reloaded and the file is not rewritten. This is a long-standing gap from the original HA/VIP work, surfaced when acknowledgements were dropped for an unrelated reason in v1.10.12. Agent-script change: sync the script from Agent Management and let the agents upgrade. No schema or API change. - **v1.10.13** (2026-08-14) — **Agent deploy acknowledgements were silently dropped** (regression in v1.10.12, fix it before or with that release): the takeover-retirement clause added to `POST /agents/{name}/keepalived-status` in v1.10.12 reused one query placeholder for both the assignment `last_deploy_hash=$n` and the comparison inside its `CASE`. PostgreSQL deduces a type per **use**, so the same placeholder came out as `text` in one and `character varying` in the other, and asyncpg rejected the statement with `AmbiguousParameterError`. The failure was not partial: the whole UPDATE never ran, so **no member ever recorded an acknowledgement**. Every VIP sat at `SYNCING (0/n)` with an empty *Last ack*, even after the nodes had deployed the config successfully, and teardown acknowledgements were lost the same way. The hash is now bound to its own placeholder, which is only ever compared against the column and therefore unambiguous. Verified against a real PostgreSQL: both statements execute, a matching hash retires the takeover authorisation, a non-matching hash and a NULL `applied_config_hash` both leave it in place, and every case records the acknowledgement. A test now asserts every `$n` in these statements is bound exactly once and that the count matches the arguments passed. Backend only: no schema, agent or API-shape change. diff --git a/UPGRADE_GUIDE.md b/UPGRADE_GUIDE.md index e0545f5..31cb463 100644 --- a/UPGRADE_GUIDE.md +++ b/UPGRADE_GUIDE.md @@ -1,3 +1,46 @@ +# Upgrade Notes — v1.11.1 (adoption cannot hide a node; Config Import on fresh installs) + +**Agent-script change, no schema change.** No `SCHEMA_VERSION` bump, so the built-in roles are +**not** re-seeded. After deploying, sync the Linux agent script from **Agent Management** and let +the agents upgrade, or none of this reaches the nodes. + +- **A node that never appeared under *Unmanaged keepalived detected* now recovers by itself.** + The agent caches the hash of its last discovery report and skips re-posting while it matches. + Delivery was judged by `curl`'s exit code, which is 0 for 5xx too, so a rejected report was + cached as delivered and a hand-maintained config — which never changes on its own — kept that + node hidden. The report is now cached only on a 2xx, and the config endpoint reports whether + the server actually holds a discovery for that agent, so the cache can only suppress while the + server agrees. **No access to the nodes is needed**; affected nodes reappear within one poll + cycle (~2.5 min) after the agents pick up the new script. +- **Permanent refusals do not loop.** A 400, 413 or 422 means the payload itself is unacceptable, + so the refusal is recorded and the same bytes are not re-posted; fixing the file releases the + brake, because it is keyed to the content hash. **401 and 404 keep retrying** — in this system + they mean a token rotation or an agent row briefly absent while it re-registers, and braking on + them would have re-created the very failure above. This matters beyond noise: 4xx and 5xx agent + calls are never sampled out of the request log, so a loop would write a row carrying the whole + `keepalived.conf` every cycle on every affected node. +- **The clear path had the same defect.** When a node becomes managed the agent tells the server + to drop the discovery; that too was judged by the exit code, so a rejected clear left a stale + row offering a **managed** node for adoption, with nothing to ever retry it. +- **Config Import now works on a freshly installed agent.** `check_config_requests` uploads a + node's live `haproxy.cfg` when you ask for it. It was defined in the installer and in the + self-upgrade daemon but not in the body a fresh install writes, and its call site is guarded by + `type`, so on such a node the feature was a silent no-op: the request was made and nothing ever + arrived. Any agent that had self-upgraded at least once already had it, which is why it went + unnoticed. A freshly installed agent now polls the pending-requests endpoint once per cycle, + exactly as every upgraded agent already does — **no node running today changes behaviour**. +- **The keepalived.conf path is resolved deterministically.** A pool may hold more than one + cluster and the join was unordered, so the path handed to an agent could differ between polls + whenever two clusters disagreed on it — the agent would inspect a file that is not there and the + node would never appear, intermittently. A customised path now wins over the shipped default, + then the lowest cluster id. With one cluster per pool, or when every cluster carries the + default, the value is byte-identical to before. + +**Rollback:** safe. No schema or data change; reverting restores the previous behaviour, in which +a rejected discovery report is never retried and Config Import is absent on fresh installs. + +--- + # Upgrade Notes — v1.11.0 (Unified request/response log) **Adds one new table and bumps `SCHEMA_VERSION` 11 → 12. The migration runs automatically on the diff --git a/backend/routers/agent.py b/backend/routers/agent.py index 85904ef..3f4b1e1 100644 --- a/backend/routers/agent.py +++ b/backend/routers/agent.py @@ -2325,16 +2325,41 @@ async def get_agent_keepalived_config(agent_name: str, x_api_key: Optional[str] # to write/own-marker-check even on not_configured/teardown. agent = await conn.fetchrow(""" SELECT a.id, a.name, COALESCE(a.enabled, TRUE) AS enabled, - hc.keepalived_config_path + hc.keepalived_config_path, + -- v1.11.1: does the server already hold a discovery for this node? The agent + -- caches the hash of its last discovery report next to the config and skips + -- re-posting while it matches. That cache used to be written even when the + -- POST was REJECTED, so a node could be hidden from the adoption panel for + -- good: the file never changes, so the agent never speaks again. Telling it + -- what we actually hold lets it recover on its own, with no extra request and + -- no one having to touch the node. + EXISTS (SELECT 1 FROM vip_discoveries vd WHERE vd.agent_id = a.id) + AS discovery_known FROM agents a LEFT JOIN haproxy_clusters hc ON hc.pool_id = a.pool_id WHERE a.name = $1 + -- A pool may hold more than one cluster, and the join then multiplies this row. With + -- no ordering the fetch took an arbitrary one, so the keepalived.conf PATH handed to + -- the agent was non-deterministic whenever two clusters in a pool disagreed on it: + -- the agent would look at the wrong file, find nothing there, and the node would + -- never appear for adoption — intermittently, which is the worst way to fail. + -- + -- A CUSTOMISED path wins over the shipped default, then the lowest cluster id. The + -- column defaults to '/etc/keepalived/keepalived.conf' rather than NULL, so ordering + -- by id alone could have picked a default-valued row over one the operator had + -- deliberately set — turning "undefined" into "reliably wrong" for that install. + -- When every cluster in the pool carries the default the string is identical, so the + -- ordering cannot change what any working deployment already receives. + ORDER BY (hc.keepalived_config_path IS NULL + OR hc.keepalived_config_path = '/etc/keepalived/keepalived.conf'), + hc.id + LIMIT 1 """, agent_name) if not agent: raise HTTPException(status_code=404, detail=f"Agent '{agent_name}' not found") config_path = agent['keepalived_config_path'] or '/etc/keepalived/keepalived.conf' if not agent['enabled']: - return {"agent_name": agent_name, "status": "not_configured", "config_path": config_path, "keepalived": None} + return {"agent_name": agent_name, "status": "not_configured", "config_path": config_path, "discovery_known": bool(agent["discovery_known"]), "keepalived": None} row = await conn.fetchrow(""" SELECT v.id AS vip_id, v.name AS vip_name, v.is_active, v.track_haproxy, @@ -2350,22 +2375,22 @@ async def get_agent_keepalived_config(agent_name: str, x_api_key: Optional[str] """, agent['id']) if not row: - return {"agent_name": agent_name, "status": "not_configured", "config_path": config_path, "keepalived": None} + return {"agent_name": agent_name, "status": "not_configured", "config_path": config_path, "discovery_known": bool(agent["discovery_known"]), "keepalived": None} if not row['is_active']: # Soft-deleted VIP → teardown. purge carries the operator's opt-in package removal; # the agent still only purges on nodes where IT installed keepalived (install marker). return {"agent_name": agent_name, "status": "teardown", "vip_id": row['vip_id'], - "config_path": config_path, "keepalived": None, + "config_path": config_path, "discovery_known": bool(agent["discovery_known"]), "keepalived": None, "purge": bool(row['purge_on_teardown'])} if not row['applied_config_content']: - return {"agent_name": agent_name, "status": "not_configured", "config_path": config_path, "keepalived": None} + return {"agent_name": agent_name, "status": "not_configured", "config_path": config_path, "discovery_known": bool(agent["discovery_known"]), "keepalived": None} from services.keepalived_config import build_haproxy_check_script check_script = build_haproxy_check_script() if row['track_haproxy'] else "" return { "agent_name": agent_name, "status": "available", - "config_path": config_path, + "config_path": config_path, "discovery_known": bool(agent["discovery_known"]), "keepalived": { "desired_state": "enabled", "install_if_missing": True, diff --git a/backend/tests/test_vip_adoption_instance.py b/backend/tests/test_vip_adoption_instance.py index 266c2ec..e3086fd 100644 --- a/backend/tests/test_vip_adoption_instance.py +++ b/backend/tests/test_vip_adoption_instance.py @@ -429,3 +429,213 @@ def test_vip_version_transition_matches_any_action(): assert 'f"vip-{vip_id}-%"' in VIP_ROUTER, ( "the PENDING -> APPLIED/REJECTED transition must match every action for the VIP" ) + + +# ---------------------------------------------------------------------------- +# v1.11.1 — a discovery report that was never accepted must be retried +# ---------------------------------------------------------------------------- + +def test_discovery_is_cached_only_when_the_server_accepted_it(): + """`curl` without -f exits 0 on 500/403/404, so the previous `if curl ...` recorded a + REJECTED discovery as delivered. The cache then suppressed every later attempt, and since + the file never changes on its own the node stayed out of the adoption panel permanently: + the only cure was deleting the cache on the node by hand.""" + script = (BACKEND / "utils" / "agent_scripts" / "linux_install.sh").read_text() + assert "if curl -k -s --connect-timeout 10 --max-time 30 -o /dev/null -X POST" not in script, ( + "the discovery POST must not be judged by curl's exit code; it is 0 for 5xx as well" + ) + assert script.count('if [[ "$disc_code" =~ ^2[0-9][0-9]$ ]]; then') == 2, ( + "both daemon copies must cache only on a 2xx" + ) + + +def test_discovery_cache_defers_to_the_server(): + """Recovery without touching the node. The server reports whether it actually holds a + discovery for this agent; only an explicit `false` overrides the cache, so a backend older + than v1.11.1 (which omits the field) keeps the previous behaviour instead of being flooded + with re-posts.""" + script = (BACKEND / "utils" / "agent_scripts" / "linux_install.sh").read_text() + agent_router = (BACKEND / "routers" / "agent.py").read_text() + + assert agent_router.count('AS discovery_known') == 1, ( + "the keepalived-config query must report whether a discovery row exists" + ) + assert agent_router.count('"discovery_known": bool(agent["discovery_known"])') == 5, ( + "every response path that knows the agent must carry the flag — the adoptable nodes are " + "precisely the not_configured ones" + ) + assert script.count('jq -r \'if has("discovery_known")') == 2 + assert script.count('[[ "$disc_known" != "false" ]] && return 0') == 2, ( + "only an explicit false may override the cache, or an older backend — which omits the " + "field — would cause a re-post on every cycle" + ) + assert script.count('conf chk disc_known=""') == 2, ( + "disc_known must be function-local; in the in-script daemon the enclosing scope is the " + "poll loop, so a stale value would outlive the response it came from" + ) + + +def test_keepalived_config_path_is_resolved_deterministically(): + """A pool may hold more than one cluster, and the join multiplies the agent row. Without an + ordering the fetch took an arbitrary cluster, so the keepalived.conf PATH handed to the agent + was non-deterministic whenever two clusters in a pool disagreed on it: the agent would look at + the wrong file, find nothing, and the node would never appear for adoption.""" + src = (BACKEND / "routers" / "agent.py").read_text() + start = src.index("async def get_agent_keepalived_config") + q_start = src.index('agent = await conn.fetchrow("""', start) + query = src[q_start:src.index('""", agent_name)', q_start)] + assert "LEFT JOIN haproxy_clusters" in query, "re-point this test; the join moved" + assert "ORDER BY" in query and "LIMIT 1" in query, ( + "the cluster row must be picked deterministically, or the config path the agent is told " + "to inspect can change between polls" + ) + assert "hc.keepalived_config_path = '/etc/keepalived/keepalived.conf'" in query, ( + "the ordering must prefer a CUSTOMISED path over the shipped default. The column defaults " + "to that path rather than NULL, so ordering by id alone could pick a default-valued row " + "over one the operator deliberately set, turning 'undefined' into 'reliably wrong'." + ) + + +def test_daemon_copies_agree_on_the_whole_keepalived_path(): + """Everything the adoption flow depends on must behave identically on BOTH install routes: + a freshly installed agent runs the heredoc body, a self-upgraded one runs the in-script + daemon. Comments may differ; logic may not.""" + import difflib + lines = (BACKEND / "utils" / "agent_scripts" / "linux_install.sh").read_text().splitlines() + term = next(i for i, l in enumerate(lines) if l.strip() == "AGENT_SCRIPT") + + def strip_comment(s): + out, q, esc = [], None, False + for ch in s: + if esc: + out.append(ch); esc = False; continue + if ch == "\\": + out.append(ch); esc = True; continue + if q: + out.append(ch) + if ch == q: + q = None + continue + if ch in ('"', "'"): + q = ch; out.append(ch); continue + if ch == "#": + break + out.append(ch) + return "".join(out).rstrip() + + def funcs(block): + found = {} + for idx, l in enumerate(block): + m = re.match(r"^(\s*)([a-zA-Z_][a-zA-Z0-9_]*)\(\)\s*\{\s*(#.*)?$", l) + if not m: + continue + close = m.group(1) + "}" + end = next((j for j in range(idx + 1, len(block)) if block[j].rstrip() == close), None) + if end is None: + continue + found[m.group(2)] = [re.sub(r"\s+", " ", strip_comment(x).strip()) + for x in block[idx + 1:end] if strip_comment(x).strip()] + return found + + here, insc = funcs(lines[923:term]), funcs(lines[term + 1:]) + for name in ("_kp_discover", "_kp_report", "_kp_teardown", + "fetch_and_deploy_keepalived_config", "get_keepalive_state"): + assert name in here and name in insc, f"{name} is missing from one daemon copy" + if here[name] != insc[name]: + d = "\n".join(x for x in difflib.unified_diff(here[name], insc[name], lineterm="") + if x[:1] in "+-" and x[:3] not in ("+++", "---")) + raise AssertionError( + f"{name}() differs between the daemon copies, so a self-upgraded agent would " + f"behave differently from a freshly installed one:\n{d}" + ) + + +def test_discovery_flag_distinguishes_false_from_absent(): + """jq's `//` returns the alternative for **false** as well as null. + + `.discovery_known // empty` therefore yields an empty string both when the backend omits the + field (older release) and when it explicitly says `false` (no discovery on record) — the one + case the recovery exists for. Written that way the fix is inert: the cache is never overridden + and a stuck node stays hidden. Caught in review, before it shipped, by parsing a real response + rather than passing the value in by hand. + """ + script = (BACKEND / "utils" / "agent_scripts" / "linux_install.sh").read_text() + assert "'.discovery_known // empty'" not in script, ( + "jq's // treats false like null, so this cannot tell 'no record' from 'old backend'" + ) + expected = ('disc_known=$(echo "$resp" | jq -r \'if has("discovery_known") ' + 'then (.discovery_known|tostring) else "" end\' 2>/dev/null)') + assert script.count(expected) == 2, ( + "both daemon copies must distinguish an explicit false from an absent field" + ) + + +def test_discovery_backs_off_on_a_permanent_rejection(): + """A 4xx means the payload itself is unacceptable, so re-posting the same bytes cannot help. + + Retrying forever is not free here: 4xx and 5xx agent calls are never sampled out of the + request log (see request_log_sink), so an unattended loop writes a row carrying the whole + keepalived.conf every poll cycle, on every affected node — the exact "polling noise evicts + the forensic record" failure the log's own defaults exist to prevent. The cache therefore + records the rejection and stays quiet until the file changes; a 5xx or a transport failure + is still retried, which is what the recovery depends on. + """ + script = (BACKEND / "utils" / "agent_scripts" / "linux_install.sh").read_text() + brake = ('elif [[ "$disc_code" == "400" || "$disc_code" == "413" ' + '|| "$disc_code" == "422" ]]; then') + assert script.count(brake) == 2, ( + "the brake must be limited to the codes that mean 'these bytes are unacceptable'" + ) + # 401 and 404 are 4xx but TRANSIENT here: a token rotation, or an agent row briefly absent + # while it re-registers. Braking on them would silence discovery for every affected node + # until its keepalived.conf changed, which for a hand-maintained file may be never — the + # exact failure this release removes. + for transient in ('"401"', '"404"'): + assert transient not in brake, ( + f"{transient} must stay in the retry class; it does not mean the payload is bad" + ) + assert script.count("""printf '%s rejected' "$cur_hash" > "$cache" 2>/dev/null""") == 2 + assert script.count('[[ "$cached_state" == "rejected" ]] && return 0') == 2, ( + "a recorded rejection must suppress the post even when the server reports no record, " + "or the flag override turns into an unbounded retry loop" + ) + # The rejection must be keyed to the CONTENT, so a fixed config is retried. + assert script.count('cached_hash="${cached_line%% *}"') == 2, ( + "the rejection is stored against the hash; changing the file must clear the brake" + ) + + +def test_config_import_reaches_a_freshly_installed_agent(): + """`check_config_requests` uploads the node's live haproxy.cfg when the operator asks for it. + + It was defined in the installer body and in the in-script daemon, but NOT in the heredoc a + fresh install writes to /usr/local/bin/haproxy-agent. Its call site is guarded by + `type check_config_requests`, so on a freshly installed agent the whole feature was a silent + no-op: the operator requested a config from the node and nothing ever arrived, with no error. + Agents that had self-upgraded at least once did have it, which is why it went unnoticed. + """ + lines = (BACKEND / "utils" / "agent_scripts" / "linux_install.sh").read_text().splitlines() + term = next(i for i, l in enumerate(lines) if l.strip() == "AGENT_SCRIPT") + pattern = re.compile(r"^\s*check_config_requests\(\)\s*\{") + + in_heredoc = sum(1 for l in lines[923:term] if pattern.match(l)) + in_daemon = sum(1 for l in lines[term + 1:] if pattern.match(l)) + assert in_heredoc == 1, ( + "the heredoc a fresh install writes must define check_config_requests, or Config Import " + "silently does nothing on any node that has never self-upgraded" + ) + assert in_daemon == 1, "the self-upgrade daemon must keep its definition" + + # And the two must be the same function, not two drifting implementations. + def body(block): + idx = next(i for i, l in enumerate(block) if pattern.match(l)) + out = [] + for l in block[idx:]: + out.append(l.strip()) + if l.strip() == "}" and len(out) > 5: + break + return out + + assert body(lines[923:term]) == body(lines[term + 1:]), ( + "the two copies of check_config_requests have drifted" + ) diff --git a/backend/utils/agent_scripts/linux_install.sh b/backend/utils/agent_scripts/linux_install.sh index 51417ba..d8f1be7 100644 --- a/backend/utils/agent_scripts/linux_install.sh +++ b/backend/utils/agent_scripts/linux_install.sh @@ -1691,7 +1691,7 @@ check_ssl_updates() { # (kept in sync with the live token in both daemon loops). fetch_and_deploy_keepalived_config() { local marker="# Managed by HAProxy OpenManager" - local resp status http_code we_own="false" conf chk + local resp status http_code we_own="false" conf chk disc_known="" # Timeouts so a hung management server can never stall the daemon loop. resp=$(curl -k -s --connect-timeout 10 --max-time 30 -w '\n%{http_code}' -X GET \ @@ -1710,6 +1710,9 @@ fetch_and_deploy_keepalived_config() { [[ -z "$conf" || "$conf" == "null" ]] && conf="/etc/keepalived/keepalived.conf" chk="$(dirname "$conf")/check_haproxy.sh" if [[ -f "$conf" ]] && grep -q "$marker" "$conf" 2>/dev/null; then we_own="true"; fi + # Whether the SERVER already holds a discovery for this node. Absent on backends older than + # v1.11.1, in which case the discovery cache keeps its previous meaning. + disc_known=$(echo "$resp" | jq -r 'if has("discovery_known") then (.discovery_known|tostring) else "" end' 2>/dev/null) # v1.10.4 — VIP adoption discovery. Report a keepalived.conf we do NOT own so an existing # VIP can be adopted from the UI instead of retyped. STRICTLY READ-ONLY: this never writes @@ -1724,28 +1727,75 @@ fetch_and_deploy_keepalived_config() { if [[ "$we_own" == "true" || ! -f "$conf" ]]; then # Nothing adoptable here. Clear a previous report exactly once. [[ -f "$cache" ]] || return 0 - curl -k -s --connect-timeout 10 --max-time 30 -X POST \ - "$MANAGEMENT_URL/api/agents/$AGENT_NAME/keepalived-discovery" \ + # Clear ONLY when the server accepted it. curl's exit code is 0 for 5xx too, so + # dropping the cache on a rejected clear lost the fact that this node is ours: the + # stale discovery row would keep offering a MANAGED node for adoption, and with the + # cache gone nothing would ever send the clear again. + local clr_code + clr_code=$(curl -k -s --connect-timeout 10 --max-time 30 -o /dev/null -w '%{http_code}' \ + -X POST "$MANAGEMENT_URL/api/agents/$AGENT_NAME/keepalived-discovery" \ -H "X-API-Key: $AGENT_TOKEN" -H "Content-Type: application/json" \ - -d "{\"config_path\":\"$conf\",\"exists\":false}" >/dev/null 2>&1 || return 0 + -d "{\"config_path\":\"$conf\",\"exists\":false}" 2>/dev/null) + [[ "$clr_code" =~ ^2[0-9][0-9]$ ]] || return 0 rm -f "$cache" return 0 fi cur_hash=$(md5sum "$conf" 2>/dev/null | awk '{print $1}') [[ -z "$cur_hash" ]] && return 0 - [[ -f "$cache" && "$(cat "$cache" 2>/dev/null)" == "$cur_hash" ]] && return 0 + # The cache may only suppress a report while the SERVER agrees it already holds one. + # v1.11.1: it suppressed unconditionally, so a report the server REJECTED was recorded + # as delivered and the node stayed out of the adoption panel for good — the file never + # changes, so nothing ever triggered another attempt and the only cure was deleting this + # file on the node by hand. `discovery_known` is absent on older backends, and only an + # explicit "false" overrides the cache, so an old server keeps the previous behaviour + # rather than being flooded with re-posts. + # Cache line is "" after a success, or " rejected" after a 4xx. + local cached_line cached_hash cached_state="" + cached_line=$(cat "$cache" 2>/dev/null) + cached_hash="${cached_line%% *}" + [[ "$cached_line" == *" "* ]] && cached_state="${cached_line#* }" + if [[ -n "$cached_hash" && "$cached_hash" == "$cur_hash" ]]; then + # A 4xx means the server refuses THESE BYTES. Re-posting them cannot succeed, and + # every attempt is stored as a failed agent call (4xx/5xx are never sampled out), so + # an unattended loop would write a row carrying the whole config every cycle. Stay + # quiet until the file changes — the reason was logged when it was rejected. + [[ "$cached_state" == "rejected" ]] && return 0 + # Otherwise the cache only holds while the SERVER agrees it has the report. + [[ "$disc_known" != "false" ]] && return 0 + fi # jq -Rs makes the file a single JSON string with its newlines intact, so the content the # server hashes is byte-identical to what is on disk — the takeover authorisation is # pinned to that hash. content_json=$(jq -Rs . < "$conf" 2>/dev/null) || return 0 body=$(jq -n --arg p "$conf" --argjson c "$content_json" \ '{config_path:$p, exists:true, is_managed:false, config_content:$c}' 2>/dev/null) || return 0 - if curl -k -s --connect-timeout 10 --max-time 30 -o /dev/null -X POST \ - "$MANAGEMENT_URL/api/agents/$AGENT_NAME/keepalived-discovery" \ + # Cache ONLY on a 2xx. curl without -f exits 0 on 500/403/404 too, so the previous + # `if curl ...` recorded a REJECTED report as delivered — and since the cache suppresses + # every later attempt until the file itself changes, one server-side error hid the node + # from the adoption panel permanently. Same failure shape as the deploy acknowledgement + # fixed in v1.10.14, on the discovery path. + local disc_code + disc_code=$(curl -k -s --connect-timeout 10 --max-time 30 -o /dev/null -w '%{http_code}' \ + -X POST "$MANAGEMENT_URL/api/agents/$AGENT_NAME/keepalived-discovery" \ -H "X-API-Key: $AGENT_TOKEN" -H "Content-Type: application/json" \ - --data-binary "$body" 2>/dev/null; then + --data-binary "$body" 2>/dev/null) + if [[ "$disc_code" =~ ^2[0-9][0-9]$ ]]; then printf '%s' "$cur_hash" > "$cache" 2>/dev/null log "INFO" "KEEPALIVED: reported an unmanaged keepalived.conf for adoption" + elif [[ "$disc_code" == "400" || "$disc_code" == "413" || "$disc_code" == "422" ]]; then + # ONLY the codes that mean "these bytes are unacceptable" stop the retry: too large + # to analyse, malformed, rejected by validation. Re-posting identical content cannot + # change any of those answers. + # + # 401 and 404 are deliberately NOT here even though they are 4xx. Both are transient + # in this system — a token rotation, or an agent row briefly absent while it + # re-registers — and treating them as permanent would silence discovery for every + # affected node until its keepalived.conf changed, which for a hand-maintained file + # may be never. That is the exact failure this release set out to remove. + printf '%s rejected' "$cur_hash" > "$cache" 2>/dev/null + log "WARN" "KEEPALIVED: discovery report refused (HTTP $disc_code); not retrying until the config changes" + else + log "WARN" "KEEPALIVED: discovery report failed (HTTP ${disc_code:-none}); will retry next cycle" fi } _kp_discover @@ -2193,6 +2243,95 @@ UPGRADE_EOF } # Check and apply configuration updates (Linux) +# Config Import: upload this node's live haproxy.cfg when the operator asks for it. +# +# This function existed ONLY in the installer body and in the in-script daemon, never in the +# heredoc that a FRESH install writes to /usr/local/bin/haproxy-agent. The call site below is +# guarded by `type check_config_requests`, so on a freshly installed agent the whole feature +# was a silent no-op: the operator requested a config from the node and nothing ever arrived, +# with no error anywhere. Agents that had self-upgraded at least once did have it, which is why +# it went unnoticed. Copied verbatim from the in-script daemon so both install routes behave +# identically (v1.11.1). +check_config_requests() { + local curl_bin="${CURL_BIN:-$(find_binary curl)}" + + # Get pending config requests from backend + log "DEBUG" "CONFIG: Checking for pending config requests from: ${MANAGEMENT_URL}/api/configuration/agents/${AGENT_NAME}/pending-requests" + + local response=$("$curl_bin" -k -s -X GET "${MANAGEMENT_URL}/api/configuration/agents/${AGENT_NAME}/pending-requests" \ + -H "Content-Type: application/json" \ + -H "X-API-Key: ${AGENT_TOKEN}") + + log "DEBUG" "CONFIG: Response: ${response:0:200}..." + + # Check if there are pending requests + local pending_count=$(echo "$response" | jq -r '.pending_requests | length' 2>/dev/null) + + log "DEBUG" "CONFIG: Pending count: $pending_count" + + if [[ "$pending_count" -gt 0 ]]; then + log "INFO" "CONFIG: Found $pending_count pending config request(s)" + + # Process each request + echo "$response" | jq -c '.pending_requests[]' 2>/dev/null | while read -r request; do + local request_id=$(echo "$request" | jq -r '.request_id') + local request_type=$(echo "$request" | jq -r '.request_type') + + log "INFO" "CONFIG: Processing config request #$request_id (type: $request_type)" + + # Read haproxy.cfg content + local config_path="${HAPROXY_CONFIG_PATH}" + if [[ -f "$config_path" ]]; then + local file_size=$(wc -c < "$config_path") + log "INFO" "CONFIG: Reading config from: $config_path (size: $file_size bytes)" + + local config_content=$(cat "$config_path") + + # Escape config content for JSON + log "DEBUG" "CONFIG: Escaping config content for JSON..." + local escaped_content=$(echo "$config_content" | jq -Rs .) + + if [[ -z "$escaped_content" ]]; then + log "ERROR" "CONFIG: JSON escaping failed for config content!" + continue + fi + + local escaped_size=${#escaped_content} + log "DEBUG" "CONFIG: JSON escaped content size: $escaped_size bytes" + + # Submit config response + local response_payload=$(cat </dev/null; then we_own="true"; fi + # See the heredoc copy. + disc_known=$(echo "$resp" | jq -r 'if has("discovery_known") then (.discovery_known|tostring) else "" end' 2>/dev/null) # v1.10.4 — VIP adoption discovery. Report a keepalived.conf we do NOT own so an existing # VIP can be adopted from the UI instead of retyped. STRICTLY READ-ONLY: this never writes @@ -3372,28 +3513,65 @@ CONFIG_RESPONSE_EOF if [[ "$we_own" == "true" || ! -f "$conf" ]]; then # Nothing adoptable here. Clear a previous report exactly once. [[ -f "$cache" ]] || return 0 - curl -k -s --connect-timeout 10 --max-time 30 -X POST \ - "$MANAGEMENT_URL/api/agents/$AGENT_NAME/keepalived-discovery" \ + # See the heredoc copy: clear only on a 2xx, or a managed node keeps being + # offered for adoption and nothing ever retries the clear. + local clr_code + clr_code=$(curl -k -s --connect-timeout 10 --max-time 30 -o /dev/null -w '%{http_code}' \ + -X POST "$MANAGEMENT_URL/api/agents/$AGENT_NAME/keepalived-discovery" \ -H "X-API-Key: $AGENT_TOKEN" -H "Content-Type: application/json" \ - -d "{\"config_path\":\"$conf\",\"exists\":false}" >/dev/null 2>&1 || return 0 + -d "{\"config_path\":\"$conf\",\"exists\":false}" 2>/dev/null) + [[ "$clr_code" =~ ^2[0-9][0-9]$ ]] || return 0 rm -f "$cache" return 0 fi cur_hash=$(md5sum "$conf" 2>/dev/null | awk '{print $1}') [[ -z "$cur_hash" ]] && return 0 - [[ -f "$cache" && "$(cat "$cache" 2>/dev/null)" == "$cur_hash" ]] && return 0 + # See the heredoc copy: the cache may only suppress while the server agrees it holds + # a discovery for this node, or a rejected report hides the node permanently. + # Cache line is "" after a success, or " rejected" after a 4xx. + local cached_line cached_hash cached_state="" + cached_line=$(cat "$cache" 2>/dev/null) + cached_hash="${cached_line%% *}" + [[ "$cached_line" == *" "* ]] && cached_state="${cached_line#* }" + if [[ -n "$cached_hash" && "$cached_hash" == "$cur_hash" ]]; then + # A 4xx means the server refuses THESE BYTES. Re-posting them cannot succeed, and + # every attempt is stored as a failed agent call (4xx/5xx are never sampled out), so + # an unattended loop would write a row carrying the whole config every cycle. Stay + # quiet until the file changes — the reason was logged when it was rejected. + [[ "$cached_state" == "rejected" ]] && return 0 + # Otherwise the cache only holds while the SERVER agrees it has the report. + [[ "$disc_known" != "false" ]] && return 0 + fi # jq -Rs makes the file a single JSON string with its newlines intact, so the content the # server hashes is byte-identical to what is on disk — the takeover authorisation is # pinned to that hash. content_json=$(jq -Rs . < "$conf" 2>/dev/null) || return 0 body=$(jq -n --arg p "$conf" --argjson c "$content_json" \ '{config_path:$p, exists:true, is_managed:false, config_content:$c}' 2>/dev/null) || return 0 - if curl -k -s --connect-timeout 10 --max-time 30 -o /dev/null -X POST \ - "$MANAGEMENT_URL/api/agents/$AGENT_NAME/keepalived-discovery" \ + # See the heredoc copy: cache ONLY on a 2xx, or a rejected report is recorded as + # delivered and the node never reappears in the adoption panel. + local disc_code + disc_code=$(curl -k -s --connect-timeout 10 --max-time 30 -o /dev/null -w '%{http_code}' \ + -X POST "$MANAGEMENT_URL/api/agents/$AGENT_NAME/keepalived-discovery" \ -H "X-API-Key: $AGENT_TOKEN" -H "Content-Type: application/json" \ - --data-binary "$body" 2>/dev/null; then + --data-binary "$body" 2>/dev/null) + if [[ "$disc_code" =~ ^2[0-9][0-9]$ ]]; then printf '%s' "$cur_hash" > "$cache" 2>/dev/null log "INFO" "KEEPALIVED: reported an unmanaged keepalived.conf for adoption" + elif [[ "$disc_code" == "400" || "$disc_code" == "413" || "$disc_code" == "422" ]]; then + # ONLY the codes that mean "these bytes are unacceptable" stop the retry: too large + # to analyse, malformed, rejected by validation. Re-posting identical content cannot + # change any of those answers. + # + # 401 and 404 are deliberately NOT here even though they are 4xx. Both are transient + # in this system — a token rotation, or an agent row briefly absent while it + # re-registers — and treating them as permanent would silence discovery for every + # affected node until its keepalived.conf changed, which for a hand-maintained file + # may be never. That is the exact failure this release set out to remove. + printf '%s rejected' "$cur_hash" > "$cache" 2>/dev/null + log "WARN" "KEEPALIVED: discovery report refused (HTTP $disc_code); not retrying until the config changes" + else + log "WARN" "KEEPALIVED: discovery report failed (HTTP ${disc_code:-none}); will retry next cycle" fi } _kp_discover diff --git a/backend/version.json b/backend/version.json index 061fdef..ab76fc6 100644 --- a/backend/version.json +++ b/backend/version.json @@ -1,5 +1,5 @@ { - "version": "1.11.0", - "releaseName": "Unified request/response log with configurable retention", - "releaseDate": "2026-08-14" + "version": "1.11.1", + "releaseName": "Adoption cannot hide a node; Config Import on fresh installs", + "releaseDate": "2026-08-15" } diff --git a/frontend/package.json b/frontend/package.json index 5f368f8..240b35a 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "haproxy-openmanager-frontend", - "version": "1.11.0", + "version": "1.11.1", "description": "HAProxy Load Balancer Management UI", "license": "AGPL-3.0-or-later", "dependencies": {