From 8ac567dfe04dd23362411d0978a6f7ca08f17983 Mon Sep 17 00:00:00 2001 From: "mustafa.ulukaya" Date: Tue, 11 Aug 2026 01:18:49 +0300 Subject: [PATCH 1/8] feat(vip): parse an existing keepalived.conf so a VIP can be adopted Groundwork for adopting a hand-maintained keepalived setup into HA/VIP management. The page is empty today because the flow is one-way: VIPs are declared in OpenManager and pushed to the node, and nothing reads what is already there. The heartbeat cannot drive adoption. It carries two keepalived facts - keepalive_state (MASTER/BACKUP, best-effort from logs) and keepalive_ip (the first address grepped out of virtual_ipaddress) - while render_keepalived_conf needs eleven: virtual_router_id, auth_pass, interface, priority, prefix_length, advert_int, unicast peers, track_haproxy, role, address and name. Guessing the rest is not a cosmetic risk: a wrong VRID puts the nodes in separate VRRP domains and a wrong auth_pass makes them reject each other, and either way both nodes claim the VIP. So the config itself has to be read. Extracting the fields is the easy half. Adoption REPLACES the operator's file with our render, so anything their file contains that the renderer cannot reproduce would be destroyed on takeover - a notify_master failover hook, an LVS virtual_server section, a sync group, a second address in one instance, a custom track_script. The parser therefore also returns everything it could not model, and build_adoption_candidate turns each entry into a blocker with the file's own line number. Values that are unknowable rather than unreproducible block too: a missing virtual_router_id, and a missing prefix length, because our renderer always writes an explicit prefix and picking one would silently change a live VIP's netmask. keepalived's own documented defaults (state BACKUP, priority 100, advert_int 1) are applied but reported in `defaulted`, so the UI can say which values were assumed rather than read. Handles the layout variation real files have: nested braces, `#` and `!` comments, blocks opened and closed on one line, quoted script paths containing spaces, and several vrrp_instance blocks in one file. A parse result carries auth_pass in cleartext, since that is the only way to re-render an identical config, so it must never be logged - noted on every function that returns one. Tests pin each blocker and the layout variants, and include the invariant that keeps the parser honest: a config the renderer itself produced must parse back with zero blockers, so adding a directive to render_keepalived_conf without teaching the parser fails the suite instead of making OpenManager's own output look unadoptable. Verified by mutation - eight deliberate weakenings of the safety checks are each caught by at least one test. No endpoint, no schema change and no agent change yet; nothing calls this. --- backend/services/keepalived_parser.py | 500 ++++++++++++++++++++++++ backend/tests/test_keepalived_parser.py | 284 ++++++++++++++ 2 files changed, 784 insertions(+) create mode 100644 backend/services/keepalived_parser.py create mode 100644 backend/tests/test_keepalived_parser.py diff --git a/backend/services/keepalived_parser.py b/backend/services/keepalived_parser.py new file mode 100644 index 0000000..3948415 --- /dev/null +++ b/backend/services/keepalived_parser.py @@ -0,0 +1,500 @@ +"""Issue #27 follow-up — parse an EXISTING keepalived.conf so a hand-maintained VIP can be +adopted into OpenManager's model (v1.10.4). + +Standalone and DB-free, like keepalived_config.py: the agent reports the file it found on a +node, this module turns it into the fields `vip_instances` / `vip_members` need, and the +adoption endpoint decides whether taking ownership is safe. + +WHY A PARSER AND NOT THE HEARTBEAT. The heartbeat carries two keepalived facts — +`keepalive_state` (MASTER/BACKUP, best-effort from logs) and `keepalive_ip` (the first +address grepped out of `virtual_ipaddress`). Rendering a node's config needs eleven: +virtual_router_id, auth_pass, interface, priority, prefix_length, advert_int, unicast +peers, track_haproxy, role and the address itself. Guessing the missing ones is not a +cosmetic risk — a wrong VRID puts the nodes in two separate VRRP domains and a wrong +auth_pass makes them reject each other, and either way both nodes claim the VIP. + +THE SAFETY CONTRACT. Adoption REPLACES the operator's file with our render, so anything in +their file that `render_keepalived_conf` cannot reproduce would be silently destroyed on +takeover — a `notify_master` failover hook, an LVS `virtual_server` section, a second +address in one instance, a sync group. Extracting the fields is the easy half; the half +that matters is `unsupported`, the list of directives we would drop. The caller must treat +a non-empty `unsupported` as a refusal to adopt, not a warning to log. + +Secrets: a parsed instance carries `auth_pass` in cleartext because that is the only way to +re-render an identical config. NEVER log a parse result. Callers persist it through +`encrypt_vrrp_secret` and mask it in anything UI-facing, exactly as the VIP router already +does for `auth_pass` in version diffs. +""" +from __future__ import annotations + +import ipaddress +import re +from typing import Any, Dict, List, Optional, Tuple + +# Directives `render_keepalived_conf` emits, and therefore the only ones a takeover can +# reproduce. Anything else found in a vrrp_instance is reported in `unsupported`. +_SUPPORTED_INSTANCE_KEYS = { + "state", "interface", "virtual_router_id", "priority", "advert_int", + "authentication", "unicast_src_ip", "unicast_peer", "virtual_ipaddress", "track_script", +} +# Top-level blocks we can account for. `vrrp_script` is reproduced only when it is the +# check script we generate ourselves (see _classify_script). +_SUPPORTED_TOP_KEYS = {"global_defs", "vrrp_script", "vrrp_instance"} + +# global_defs entries our render emits. An operator's file usually carries more (notification +# email, router_id, ...) and losing those is a real change, so they are reported too. +_SUPPORTED_GLOBAL_KEYS = {"enable_script_security", "script_user"} + +_IDENT_RE = re.compile(r"^[A-Za-z0-9._:-]+$") + + +class KeepalivedParseError(ValueError): + """The text is not a keepalived.conf we can reason about (unbalanced braces etc.).""" + + +# --------------------------------------------------------------------------- +# Tokenizer / block reader +# --------------------------------------------------------------------------- +def _strip_comment(line: str) -> str: + """Drop a trailing comment. keepalived treats BOTH `#` and `!` as comment starters, and + neither is meaningful inside the quoted script paths we care about, so a quote-aware + scan is enough (a `#` inside quotes stays).""" + out: List[str] = [] + quote: Optional[str] = None + for ch in line: + if quote: + out.append(ch) + if ch == quote: + quote = None + continue + if ch in ('"', "'"): + quote = ch + out.append(ch) + continue + if ch in ("#", "!"): + break + out.append(ch) + return "".join(out) + + +def _split_tokens(line: str) -> List[str]: + """Whitespace split that keeps quoted strings whole and isolates braces, so + `virtual_ipaddress { 10.0.0.1/24 dev eth0 }` tokenizes the same as its multi-line form.""" + tokens: List[str] = [] + buf: List[str] = [] + quote: Optional[str] = None + + def flush() -> None: + if buf: + tokens.append("".join(buf)) + buf.clear() + + for ch in line: + if quote: + if ch == quote: + quote = None + else: + buf.append(ch) + continue + if ch in ('"', "'"): + quote = ch + continue + if ch.isspace(): + flush() + elif ch in ("{", "}"): + flush() + tokens.append(ch) + else: + buf.append(ch) + flush() + return tokens + + +def _read_blocks(text: str) -> List[Dict[str, Any]]: + """Parse the file into nested entries. + + Each entry is either + {"kind": "block", "name": str, "args": [str], "body": [entries], "line": int} + {"kind": "line", "tokens": [str], "line": int} + + Line boundaries matter: inside `virtual_ipaddress` and `unicast_peer` each line is one + bare value, so a flat token stream could not tell two addresses apart. + """ + root: List[Dict[str, Any]] = [] + stack: List[List[Dict[str, Any]]] = [root] + # Blocks whose opening `{` we have seen, so a stray `}` can be reported with context. + open_blocks: List[str] = [] + + for lineno, raw in enumerate(text.splitlines(), start=1): + pending: List[str] = [] + for tok in _split_tokens(_strip_comment(raw)): + if tok == "{": + name = pending[0] if pending else "" + args = pending[1:] + block = {"kind": "block", "name": name, "args": args, "body": [], "line": lineno} + stack[-1].append(block) + stack.append(block["body"]) + open_blocks.append(name) + pending = [] + elif tok == "}": + if pending: + stack[-1].append({"kind": "line", "tokens": pending, "line": lineno}) + pending = [] + if len(stack) == 1: + raise KeepalivedParseError(f"unbalanced '}}' on line {lineno}") + stack.pop() + open_blocks.pop() + else: + pending.append(tok) + if pending: + stack[-1].append({"kind": "line", "tokens": pending, "line": lineno}) + + if len(stack) != 1: + raise KeepalivedParseError(f"unclosed block '{open_blocks[-1] or '?'}' at end of file") + return root + + +# --------------------------------------------------------------------------- +# Interpretation +# --------------------------------------------------------------------------- +def _as_int(tokens: List[str]) -> Optional[int]: + if len(tokens) < 2: + return None + try: + return int(tokens[1]) + except (TypeError, ValueError): + return None + + +def _parse_vip_entry(tokens: List[str]) -> Optional[Dict[str, Any]]: + """One `virtual_ipaddress` line: `[/] [dev ] [label ...]`. + + Returns None when the first token is not an address — a shape we do not understand must + surface as unsupported rather than be silently dropped. + """ + spec = tokens[0] + addr, _, prefix = spec.partition("/") + try: + ip = ipaddress.ip_address(addr) + except ValueError: + return None + entry: Dict[str, Any] = { + "address": str(ip), + "prefix_length": None, + "dev": None, + "extra": [], + } + if prefix: + try: + entry["prefix_length"] = int(prefix) + except ValueError: + return None + rest = tokens[1:] + i = 0 + while i < len(rest): + if rest[i] == "dev" and i + 1 < len(rest): + entry["dev"] = rest[i + 1] + i += 2 + continue + # `label`, `scope`, `brd`, ... — all real directives we do not render. + entry["extra"].append(rest[i]) + i += 1 + return entry + + +def _classify_script(block: Dict[str, Any]) -> Tuple[str, Optional[str]]: + """Return (name, script_path) for a vrrp_script block.""" + name = block["args"][0] if block["args"] else (block["name"] or "") + path = None + for entry in block["body"]: + if entry["kind"] == "line" and entry["tokens"] and entry["tokens"][0] == "script": + path = " ".join(entry["tokens"][1:]) or None + return name, path + + +def _parse_instance(block: Dict[str, Any]) -> Dict[str, Any]: + """Interpret one `vrrp_instance` block into VIP-model fields plus its own unsupported list.""" + inst: Dict[str, Any] = { + "instance_name": block["args"][0] if block["args"] else "", + "state": None, + "interface": None, + "virtual_router_id": None, + "priority": None, + "advert_int": None, + "auth_type": None, + "auth_pass": None, + "unicast_src_ip": None, + "unicast_peers": [], + "virtual_ips": [], + "track_scripts": [], + "unsupported": [], + "line": block["line"], + } + + def unsupported(what: str, lineno: int) -> None: + inst["unsupported"].append({"directive": what, "line": lineno}) + + for entry in block["body"]: + if entry["kind"] == "line": + tokens = entry["tokens"] + key = tokens[0] + if key == "state": + inst["state"] = (tokens[1].upper() if len(tokens) > 1 else None) + elif key == "interface": + inst["interface"] = tokens[1] if len(tokens) > 1 else None + elif key == "virtual_router_id": + inst["virtual_router_id"] = _as_int(tokens) + elif key == "priority": + inst["priority"] = _as_int(tokens) + elif key == "advert_int": + # keepalived accepts sub-second floats; our model column is an integer. + raw = tokens[1] if len(tokens) > 1 else "" + try: + val = float(raw) + except (TypeError, ValueError): + val = None + if val is None: + unsupported(f"advert_int {raw}", entry["line"]) + elif val != int(val): + # Rounding would change VRRP timing, so refuse rather than adopt-and-alter. + unsupported(f"advert_int {raw} (fractional; model stores whole seconds)", + entry["line"]) + else: + inst["advert_int"] = int(val) + elif key == "unicast_src_ip": + inst["unicast_src_ip"] = tokens[1] if len(tokens) > 1 else None + else: + unsupported(" ".join(tokens), entry["line"]) + continue + + name = entry["name"] + if name == "authentication": + for sub in entry["body"]: + if sub["kind"] != "line" or not sub["tokens"]: + continue + k = sub["tokens"][0] + if k == "auth_type": + inst["auth_type"] = (sub["tokens"][1].upper() if len(sub["tokens"]) > 1 else None) + elif k == "auth_pass": + # Everything after the keyword: a VRRP password may contain spaces. + inst["auth_pass"] = " ".join(sub["tokens"][1:]) or None + else: + unsupported(f"authentication/{' '.join(sub['tokens'])}", sub["line"]) + elif name == "unicast_peer": + for sub in entry["body"]: + if sub["kind"] == "line" and sub["tokens"]: + inst["unicast_peers"].append(sub["tokens"][0]) + else: + unsupported("unicast_peer/", entry["line"]) + elif name == "virtual_ipaddress": + for sub in entry["body"]: + if sub["kind"] != "line" or not sub["tokens"]: + unsupported("virtual_ipaddress/", entry["line"]) + continue + parsed = _parse_vip_entry(sub["tokens"]) + if parsed is None: + unsupported(f"virtual_ipaddress/{' '.join(sub['tokens'])}", sub["line"]) + else: + if parsed["extra"]: + unsupported( + f"virtual_ipaddress/{parsed['address']} " + f"({' '.join(parsed['extra'])})", sub["line"]) + inst["virtual_ips"].append(parsed) + elif name == "track_script": + for sub in entry["body"]: + if sub["kind"] == "line" and sub["tokens"]: + inst["track_scripts"].append(sub["tokens"][0]) + else: + unsupported(f"{name} {{...}}", entry["line"]) + + return inst + + +def parse_keepalived_conf(text: str) -> Dict[str, Any]: + """Parse a keepalived.conf into VIP-model fields plus everything we could not model. + + Raises KeepalivedParseError on structurally broken input. Never log the result: parsed + instances carry `auth_pass` in cleartext. + """ + root = _read_blocks(text or "") + result: Dict[str, Any] = { + "instances": [], + "scripts": {}, + "global_defs": {}, + "unsupported": [], # top-level directives our render would drop + "sync_groups": [], + } + + for entry in root: + if entry["kind"] == "line": + # A bare top-level directive (e.g. `include /etc/keepalived/conf.d/*.conf`). + result["unsupported"].append( + {"directive": " ".join(entry["tokens"]), "line": entry["line"]}) + continue + name = entry["name"] + if name == "global_defs": + for sub in entry["body"]: + if sub["kind"] == "line" and sub["tokens"]: + key = sub["tokens"][0] + result["global_defs"][key] = " ".join(sub["tokens"][1:]) + if key not in _SUPPORTED_GLOBAL_KEYS: + result["unsupported"].append( + {"directive": f"global_defs/{' '.join(sub['tokens'])}", + "line": sub["line"]}) + else: + result["unsupported"].append( + {"directive": f"global_defs/{sub.get('name', '?')} {{...}}", + "line": sub["line"]}) + elif name == "vrrp_script": + script_name, path = _classify_script(entry) + result["scripts"][script_name] = {"script": path, "line": entry["line"]} + elif name == "vrrp_instance": + result["instances"].append(_parse_instance(entry)) + elif name == "vrrp_sync_group": + # A sync group ties instances together so they fail over as a unit. Our render has + # no equivalent, and dropping it changes failover semantics — never adopt silently. + group = entry["args"][0] if entry["args"] else "" + result["sync_groups"].append({"name": group, "line": entry["line"]}) + result["unsupported"].append( + {"directive": f"vrrp_sync_group {group}", "line": entry["line"]}) + else: + # virtual_server (LVS), static_routes, bfd_instance, ... + args = " ".join(entry["args"]) + result["unsupported"].append( + {"directive": f"{name} {args} {{...}}".replace(" ", " "), "line": entry["line"]}) + + return result + + +# --------------------------------------------------------------------------- +# Mapping to the VIP model + the adoption gate +# --------------------------------------------------------------------------- +# keepalived defaults we are willing to apply when a directive is absent, because the value +# is unambiguous and re-rendering it changes nothing on the wire. +_DEFAULT_ADVERT_INT = 1 +_DEFAULT_PRIORITY = 100 +_DEFAULT_STATE = "BACKUP" + +# The only track_script our renderer emits (keepalived_config.build_haproxy_check_script). +OUR_CHECK_SCRIPT_NAME = "chk_haproxy" + + +def build_adoption_candidate(parsed: Dict[str, Any], instance: Dict[str, Any]) -> Dict[str, Any]: + """Map one parsed `vrrp_instance` onto vip_instances / vip_members fields. + + Returns `adoptable` plus `blockers`. A blocker means taking ownership would change what + is running — either because our render cannot reproduce something in the file, or because + a value we must write is not knowable from the file. Adoption REPLACES the operator's + config, so "we could not read it" and "we would change it" are the same hazard, and both + have to stop the flow rather than be logged. + + Never log the return value: `vip.auth_pass` is cleartext. + """ + blockers: List[str] = [] + + # Directives we would drop. Report the file's own line numbers so the operator can look. + dropped = list(parsed.get("unsupported") or []) + list(instance.get("unsupported") or []) + for d in dropped: + blockers.append( + f"line {d['line']}: `{d['directive']}` — OpenManager's renderer cannot reproduce " + f"this, so adopting would delete it") + + vips = instance.get("virtual_ips") or [] + if len(vips) == 0: + blockers.append("the instance declares no virtual_ipaddress — nothing to adopt") + elif len(vips) > 1: + addrs = ", ".join(v["address"] for v in vips) + blockers.append( + f"the instance carries {len(vips)} addresses ({addrs}); a managed VIP holds exactly " + f"one, so adopting would drop all but the first") + + vip_entry = vips[0] if vips else None + + if instance.get("virtual_router_id") is None: + blockers.append("no virtual_router_id — it cannot be guessed: a wrong VRID puts the " + "nodes in separate VRRP domains and both would claim the VIP") + if not instance.get("interface"): + blockers.append("no interface — required to render the instance and the address") + + # An explicit prefix is required. Our renderer ALWAYS writes `/`, the model + # column defaults to 24, and keepalived's own default for a bare address is a host route. + # Picking either one for the operator would silently change the VIP's netmask, so ask. + if vip_entry is not None and vip_entry.get("prefix_length") is None: + blockers.append( + f"`{vip_entry['address']}` has no explicit prefix length; state it during adoption " + f"so the netmask cannot change on takeover") + + # The address must live on the instance's interface — that is the only `dev` we can render. + if vip_entry is not None and vip_entry.get("dev") and instance.get("interface") \ + and vip_entry["dev"] != instance["interface"]: + blockers.append( + f"the address is bound to `dev {vip_entry['dev']}` but the instance uses " + f"`interface {instance['interface']}`; the render always uses the instance interface") + + auth_type = instance.get("auth_type") + if auth_type not in (None, "PASS"): + blockers.append(f"auth_type {auth_type} is not supported (only PASS is rendered)") + + # A tracked script that is not ours would be replaced by our HAProxy check. + tracked = [t for t in (instance.get("track_scripts") or [])] + foreign = [t for t in tracked if t != OUR_CHECK_SCRIPT_NAME] + if foreign: + blockers.append( + f"track_script {', '.join(foreign)} would be replaced by OpenManager's HAProxy " + f"health check") + + state = instance.get("state") or _DEFAULT_STATE + if state not in ("MASTER", "BACKUP"): + blockers.append(f"state {state} is not MASTER or BACKUP") + + peers = list(instance.get("unicast_peers") or []) + src = instance.get("unicast_src_ip") + # Our renderer emits unicast_src_ip and unicast_peer together, or neither. + if bool(src) != bool(peers): + which = "unicast_src_ip without unicast_peer" if src else "unicast_peer without unicast_src_ip" + blockers.append(f"{which} — the render emits both or neither") + + candidate: Dict[str, Any] = { + "instance_name": instance.get("instance_name") or "", + "adoptable": not blockers, + "blockers": blockers, + "dropped_directives": dropped, + "vip": { + "virtual_ip": vip_entry["address"] if vip_entry else None, + "prefix_length": vip_entry.get("prefix_length") if vip_entry else None, + "virtual_router_id": instance.get("virtual_router_id"), + "advert_int": instance.get("advert_int") if instance.get("advert_int") is not None + else _DEFAULT_ADVERT_INT, + "use_unicast": bool(peers), + "track_haproxy": OUR_CHECK_SCRIPT_NAME in tracked, + "auth_pass": instance.get("auth_pass"), + }, + "member": { + "network_interface": instance.get("interface"), + "role": state, + "priority": instance.get("priority") if instance.get("priority") is not None + else _DEFAULT_PRIORITY, + }, + "peers": peers, + "unicast_src_ip": src, + # Which values came from a keepalived default rather than the file, so the UI can say so. + "defaulted": [ + k for k, present in ( + ("advert_int", instance.get("advert_int") is not None), + ("priority", instance.get("priority") is not None), + ("state", instance.get("state") is not None), + ) if not present + ], + } + return candidate + + +def analyse_keepalived_conf(text: str) -> Dict[str, Any]: + """Parse + map in one call: the shape the discovery endpoint stores and the UI renders.""" + parsed = parse_keepalived_conf(text) + return { + "instance_count": len(parsed["instances"]), + "sync_groups": parsed["sync_groups"], + "global_defs": parsed["global_defs"], + "candidates": [build_adoption_candidate(parsed, inst) for inst in parsed["instances"]], + } diff --git a/backend/tests/test_keepalived_parser.py b/backend/tests/test_keepalived_parser.py new file mode 100644 index 0000000..0b5d92d --- /dev/null +++ b/backend/tests/test_keepalived_parser.py @@ -0,0 +1,284 @@ +"""Issue #27 follow-up (v1.10.4) — unit tests for parsing an EXISTING keepalived.conf so a +hand-maintained VIP can be adopted. + +Pure-function tests; no DB, no network. The parser exists because the heartbeat carries only +the VIP address and a best-effort MASTER/BACKUP, while rendering a node's config needs eleven +fields — and because adoption REPLACES the operator's file, so anything our renderer cannot +reproduce has to be reported as a blocker rather than silently dropped. +""" +from __future__ import annotations + +import os +import sys + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +os.environ.setdefault("SECRET_KEY", "test-secret-key-for-keepalived-parser-tests") + +from services import keepalived_config as kc # noqa: E402 +from services.keepalived_parser import ( # noqa: E402 + KeepalivedParseError, analyse_keepalived_conf, build_adoption_candidate, + parse_keepalived_conf, +) + + +# A realistic hand-maintained config: two nodes, unicast VRRP, password auth, HAProxy check. +HANDWRITTEN = """\ +! Configuration File for keepalived +global_defs { + enable_script_security + script_user root +} + +vrrp_script chk_haproxy { + script "/etc/keepalived/check_haproxy.sh" + interval 2 + weight -21 +} + +vrrp_instance VI_1 { + state MASTER + interface eth0 # public leg + virtual_router_id 51 + priority 150 + advert_int 1 + authentication { + auth_type PASS + auth_pass s3cr3t + } + unicast_src_ip 10.0.0.11 + unicast_peer { + 10.0.0.12 + } + virtual_ipaddress { + 10.0.0.100/24 dev eth0 + } + track_script { + chk_haproxy + } +} +""" + + +def _only_candidate(text): + parsed = parse_keepalived_conf(text) + assert len(parsed["instances"]) == 1 + return build_adoption_candidate(parsed, parsed["instances"][0]) + + +def test_parses_a_handwritten_config_into_model_fields(): + cand = _only_candidate(HANDWRITTEN) + assert cand["adoptable"] is True, cand["blockers"] + assert cand["blockers"] == [] + assert cand["vip"] == { + "virtual_ip": "10.0.0.100", + "prefix_length": 24, + "virtual_router_id": 51, + "advert_int": 1, + "use_unicast": True, + "track_haproxy": True, + "auth_pass": "s3cr3t", + } + assert cand["member"] == {"network_interface": "eth0", "role": "MASTER", "priority": 150} + assert cand["peers"] == ["10.0.0.12"] and cand["unicast_src_ip"] == "10.0.0.11" + assert cand["defaulted"] == [] # every value came from the file, nothing assumed + + +def test_comment_and_layout_variants(): + # `!` and `#` both start comments; a block may open and close on one line; a quoted + # script path keeps its spaces. None of this may change the parse. + text = """\ +#!/not/a/shebang — this whole line is a comment +vrrp_script chk { script "/opt/my scripts/chk.sh" } +vrrp_instance VI_1 { state BACKUP + interface eth1 ! trailing bang comment + virtual_router_id 7 + priority 90 + virtual_ipaddress { 192.168.5.9/32 dev eth1 } +} +""" + parsed = parse_keepalived_conf(text) + assert parsed["scripts"]["chk"]["script"] == "/opt/my scripts/chk.sh" + inst = parsed["instances"][0] + assert inst["state"] == "BACKUP" and inst["interface"] == "eth1" + assert inst["virtual_router_id"] == 7 and inst["priority"] == 90 + assert inst["virtual_ips"] == [ + {"address": "192.168.5.9", "prefix_length": 32, "dev": "eth1", "extra": []} + ] + + +def test_documented_defaults_are_applied_and_flagged(): + # keepalived's own defaults for absent directives. Applying them re-renders the same + # behaviour, so they are allowed — but the UI must be able to say they were assumed. + text = """\ +vrrp_instance VI_1 { + interface eth0 + virtual_router_id 12 + virtual_ipaddress { 10.1.1.5/24 dev eth0 } +} +""" + cand = _only_candidate(text) + assert cand["adoptable"] is True, cand["blockers"] + assert cand["member"]["role"] == "BACKUP" and cand["member"]["priority"] == 100 + assert cand["vip"]["advert_int"] == 1 + assert sorted(cand["defaulted"]) == ["advert_int", "priority", "state"] + # No authentication block and no track_script — both legal, both faithfully represented. + assert cand["vip"]["auth_pass"] is None and cand["vip"]["track_haproxy"] is False + assert cand["vip"]["use_unicast"] is False + + +def _blockers_for(text): + return " | ".join(_only_candidate(text)["blockers"]) + + +def test_directives_we_cannot_render_block_adoption(): + # THE central safety property: adoption overwrites the file, so a failover hook we do not + # render would be destroyed. It must stop the flow, not warn. + text = HANDWRITTEN.replace( + " track_script {", ' notify_master "/usr/local/bin/promote.sh"\n track_script {') + blockers = _blockers_for(text) + assert "notify_master" in blockers and "would delete it" in blockers + assert _only_candidate(text)["adoptable"] is False + + +def test_multiple_addresses_in_one_instance_block_adoption(): + text = HANDWRITTEN.replace(" 10.0.0.100/24 dev eth0", + " 10.0.0.100/24 dev eth0\n 10.0.0.101/24 dev eth0") + blockers = _blockers_for(text) + assert "2 addresses" in blockers and "10.0.0.101" in blockers + + +def test_missing_vrid_blocks_adoption_with_the_split_brain_reason(): + text = HANDWRITTEN.replace(" virtual_router_id 51\n", "") + blockers = _blockers_for(text) + assert "no virtual_router_id" in blockers and "separate VRRP domains" in blockers + + +def test_missing_prefix_blocks_adoption(): + # Our renderer always writes an explicit prefix; guessing one would change the netmask of a + # live VIP, so the operator has to state it. + text = HANDWRITTEN.replace("10.0.0.100/24 dev eth0", "10.0.0.100 dev eth0") + blockers = _blockers_for(text) + assert "no explicit prefix length" in blockers + + +def test_address_on_a_different_dev_blocks_adoption(): + text = HANDWRITTEN.replace("10.0.0.100/24 dev eth0", "10.0.0.100/24 dev eth1") + blockers = _blockers_for(text) + assert "dev eth1" in blockers and "interface eth0" in blockers + + +def test_foreign_track_script_blocks_adoption(): + text = HANDWRITTEN.replace(" chk_haproxy", " chk_custom") + blockers = _blockers_for(text) + assert "chk_custom" in blockers and "replaced by OpenManager" in blockers + + +def test_unsupported_auth_type_blocks_adoption(): + text = HANDWRITTEN.replace("auth_type PASS", "auth_type AH") + assert "auth_type AH" in _blockers_for(text) + + +def test_fractional_advert_int_blocks_adoption(): + # Rounding 0.5s to 1s changes VRRP timing, so adopt-and-alter is not acceptable. + text = HANDWRITTEN.replace("advert_int 1", "advert_int 0.5") + blockers = _blockers_for(text) + assert "advert_int 0.5" in blockers and "fractional" in blockers + + +def test_half_configured_unicast_blocks_adoption(): + text = HANDWRITTEN.replace(" unicast_peer {\n 10.0.0.12\n }\n", "") + assert "unicast_src_ip without unicast_peer" in _blockers_for(text) + + +def test_sync_group_and_lvs_sections_block_adoption(): + text = HANDWRITTEN + """ +vrrp_sync_group VG1 { + group { + VI_1 + } +} +virtual_server 10.0.0.100 80 { + lb_algo rr +} +""" + parsed = parse_keepalived_conf(text) + assert [g["name"] for g in parsed["sync_groups"]] == ["VG1"] + directives = " ".join(d["directive"] for d in parsed["unsupported"]) + assert "vrrp_sync_group VG1" in directives and "virtual_server" in directives + # Both are top-level, so EVERY candidate in the file is blocked — a sync group changes + # failover semantics for the instances it groups. + cand = build_adoption_candidate(parsed, parsed["instances"][0]) + assert cand["adoptable"] is False + + +def test_extra_global_defs_are_reported_as_losses(): + text = HANDWRITTEN.replace(" script_user root", + " script_user root\n router_id LVS_DEVEL") + parsed = parse_keepalived_conf(text) + directives = " ".join(d["directive"] for d in parsed["unsupported"]) + assert "global_defs/router_id LVS_DEVEL" in directives + assert parsed["global_defs"]["router_id"] == "LVS_DEVEL" + + +def test_multiple_instances_yield_one_candidate_each(): + text = HANDWRITTEN + """ +vrrp_instance VI_2 { + state BACKUP + interface eth0 + virtual_router_id 52 + priority 100 + advert_int 1 + virtual_ipaddress { 10.0.0.200/24 dev eth0 } +} +""" + analysed = analyse_keepalived_conf(text) + assert analysed["instance_count"] == 2 + names = [c["instance_name"] for c in analysed["candidates"]] + assert names == ["VI_1", "VI_2"] + assert [c["vip"]["virtual_ip"] for c in analysed["candidates"]] == ["10.0.0.100", "10.0.0.200"] + assert all(c["adoptable"] for c in analysed["candidates"]) + + +def test_unbalanced_braces_raise(): + for bad in ("vrrp_instance VI_1 {\n state MASTER\n", "}\n"): + raised = False + try: + parse_keepalived_conf(bad) + except KeepalivedParseError: + raised = True + assert raised, f"should have raised for {bad!r}" + + +def test_our_own_render_round_trips_with_zero_blockers(): + """The invariant that keeps the parser honest: a config WE generated must parse back into + the same model with nothing unsupported. If a future change to render_keepalived_conf emits + a directive the parser does not know, this fails — instead of adoption silently reporting + that OpenManager's own output is unadoptable.""" + vip = {"id": 3, "name": "web-vip", "virtual_ip": "10.0.0.100", "prefix_length": 24, + "virtual_router_id": 51, "advert_int": 1, "use_unicast": True, "track_haproxy": True} + members = [{"role": "MASTER", "priority": 150, "network_interface": "eth0", + "agent_id": 1, "ip_address": "10.0.0.11"}, + {"role": "BACKUP", "priority": 100, "network_interface": "eth0", + "agent_id": 2, "ip_address": "10.0.0.12"}] + rendered = kc.render_keepalived_conf( + vip=vip, members=members, this_agent=members[0], + peer_ips=["10.0.0.12"], auth_pass_plain="s3cr3t") + + cand = _only_candidate(rendered) + assert cand["adoptable"] is True, cand["blockers"] + assert cand["vip"]["virtual_ip"] == vip["virtual_ip"] + assert cand["vip"]["prefix_length"] == vip["prefix_length"] + assert cand["vip"]["virtual_router_id"] == vip["virtual_router_id"] + assert cand["vip"]["track_haproxy"] is True and cand["vip"]["use_unicast"] is True + assert cand["member"] == {"network_interface": "eth0", "role": "MASTER", "priority": 150} + assert cand["vip"]["auth_pass"] == "s3cr3t" + + # And the same for the no-auth / multicast / untracked shape, which renders fewer blocks. + plain = kc.render_keepalived_conf( + vip={**vip, "use_unicast": False, "track_haproxy": False}, + members=members, this_agent=members[1], peer_ips=[], auth_pass_plain=None) + cand2 = _only_candidate(plain) + assert cand2["adoptable"] is True, cand2["blockers"] + assert cand2["vip"]["use_unicast"] is False and cand2["vip"]["track_haproxy"] is False + assert cand2["vip"]["auth_pass"] is None From 7dfd31832af63a4fbffd7f5c957b938bdd0130ce Mon Sep 17 00:00:00 2001 From: "mustafa.ulukaya" Date: Tue, 11 Aug 2026 01:35:59 +0300 Subject: [PATCH 2/8] feat(vip): store the keepalived.conf an agent finds on a node Ingest side of adopting an existing VIP. A node reports the keepalived.conf it found and does NOT own to a new endpoint, and the finding is kept in a new vip_discoveries table (one row per agent, since the file is per-node). Reporting is read-only on the node. The heartbeat cannot carry this: it has the VIP address and a best-effort MASTER/BACKUP, while rendering a node's config needs eleven fields, so the file itself has to be read and parsed server-side - parsing keepalived's block syntax in bash is not something to attempt on a production load balancer. Secrets are split at ingest. The reported content may contain the VRRP auth_pass, so the password is Fernet-encrypted into its own column through the same key path as vip_instances, and the stored copy of the file has it masked. Nothing readable through the API, the UI preview or a database dump carries it in cleartext, and the parse result is never logged. A file that does not parse records its error rather than failing the agent's poll loop, and a report of "the file is gone" clears the row so the UI stops offering a stale candidate. The keepalived-config delivery gains allow_takeover and takeover_expected_hash. The agent refuses to overwrite a keepalived.conf without our ownership marker, which is the guard that protects a hand-maintained setup - and is exactly the guard adoption has to pass. Rather than weaken it, an adopted VIP authorises exactly ONE takeover of exactly the file that was analysed by pinning its md5, so a config edited between adoption and Apply is still refused instead of being silently overwritten. SCHEMA_VERSION 10 -> 11 for the new table and two additive columns. Nothing existing is altered, but the bump re-seeds the four built-in roles, which the upgrade notes call out. --- backend/database/migrations.py | 47 +++++++++++++- backend/routers/agent.py | 113 ++++++++++++++++++++++++++++++++- 2 files changed, 157 insertions(+), 3 deletions(-) diff --git a/backend/database/migrations.py b/backend/database/migrations.py index de0f437..926bfbd 100644 --- a/backend/database/migrations.py +++ b/backend/database/migrations.py @@ -1758,7 +1758,13 @@ async def ensure_agent_activity_logs_table(): # until the operator imports the CA-signed certificate; the import creates a # normal ssl_certificates row and NULLs the key copy here. Additive + idempotent; # no existing table is altered, agents never read this table. -SCHEMA_VERSION = 10 +# v1.10.4 (VIP adoption): bumped 10 -> 11 for the new `vip_discoveries` table plus two +# additive columns (`vip_instances.adopted_at`, `vip_members.takeover_expected_hash`). +# Holds the keepalived.conf an agent found already on a node so an existing VIP can be +# adopted instead of retyped. Additive + idempotent; no existing table is altered and no +# existing row changes. NOTE for the upgrade notes: a SCHEMA_VERSION bump re-seeds the four +# built-in roles to their defaults, so role customizations are lost on this upgrade. +SCHEMA_VERSION = 11 async def run_all_migrations(): @@ -2161,6 +2167,45 @@ async def ensure_vip_tables(): "CREATE INDEX IF NOT EXISTS idx_vip_members_agent ON vip_members(agent_id);" ) + # ── v1.10.4 — VIP adoption: what the agent found already on the node ────────── + # A node with a hand-maintained keepalived.conf reports it here so an existing VIP can + # be adopted instead of retyped. One row per agent (the file is per-node); the agent + # only reports a config it does NOT own, and only when the content changed. + # + # SECRETS: `raw_config` is stored MASKED (auth_pass replaced) because it is served to + # the UI. The real VRRP password is Fernet-encrypted in auth_pass_encrypted, mirroring + # vip_instances, so adoption can carry it into the managed VIP without it ever being + # readable through the API or a DB dump. `analysis` is the parser output with auth_pass + # stripped out. + await conn.execute(""" + CREATE TABLE IF NOT EXISTS vip_discoveries ( + id SERIAL PRIMARY KEY, + agent_id INTEGER NOT NULL REFERENCES agents(id) ON DELETE CASCADE, + config_path VARCHAR(500) NOT NULL, + config_hash VARCHAR(64) NOT NULL, + is_managed BOOLEAN NOT NULL DEFAULT FALSE, + raw_config_masked TEXT, + auth_pass_encrypted TEXT, + analysis JSONB, + parse_error TEXT, + adopted_vip_id INTEGER REFERENCES vip_instances(id) ON DELETE SET NULL, + reported_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + CONSTRAINT vip_discovery_agent_unique UNIQUE (agent_id) + ); + """) + await conn.execute( + "CREATE INDEX IF NOT EXISTS idx_vip_discoveries_agent ON vip_discoveries(agent_id);" + ) + # Adoption provenance + the one-shot takeover authorisation. The agent refuses to + # overwrite a keepalived.conf that lacks our ownership marker, which is exactly the + # guard adoption has to pass. Rather than weaken it, an adopted VIP carries the hash of + # the file we analysed: the agent takes over ONLY if the file on disk still hashes to + # that value, so a config that changed after adoption is never clobbered. + await conn.execute( + "ALTER TABLE vip_instances ADD COLUMN IF NOT EXISTS adopted_at TIMESTAMP;") + await conn.execute( + "ALTER TABLE vip_members ADD COLUMN IF NOT EXISTS takeover_expected_hash VARCHAR(64);") + logger.info("✅ VIP tables ensured (Issue #27 — HA/VIP Keepalived management)") except Exception as e: logger.error(f"Failed to ensure VIP tables: {e}") diff --git a/backend/routers/agent.py b/backend/routers/agent.py index 671462e..b3eebec 100644 --- a/backend/routers/agent.py +++ b/backend/routers/agent.py @@ -9,8 +9,15 @@ import os import json import ipaddress import hashlib +import re # Pipeline trigger - force backend redeploy v2 +# v1.10.4 — a discovered keepalived.conf is stored and served to the UI, so the VRRP password is +# masked out of the stored copy (the real value lives Fernet-encrypted in its own column). Mask +# the WHOLE remainder of the line, mirroring vip.py's version-diff masking, so a password +# containing whitespace cannot partially leak. +_AUTH_PASS_MASK_RE = re.compile(r"(auth_pass\s+).*") + from models import AgentCreate from models.agent import AgentToggle, AgentHeartbeat, AgentScriptRequest, AgentUpgradeRequest from database.connection import get_database_connection, close_database_connection @@ -26,7 +33,7 @@ logger = logging.getLogger(__name__) # Global version storage (acts as in-memory database) AGENT_VERSIONS = { "macos": "2.1.0", # Updated via endpoint - "linux": "2.0.0" + "linux": "2.1.0" } @@ -2332,7 +2339,7 @@ async def get_agent_keepalived_config(agent_name: str, x_api_key: Optional[str] row = await conn.fetchrow(""" SELECT v.id AS vip_id, v.name AS vip_name, v.is_active, v.track_haproxy, v.purge_on_teardown, - m.applied_config_content, m.applied_config_hash + m.applied_config_content, m.applied_config_hash, m.takeover_expected_hash FROM vip_members m JOIN vip_instances v ON v.id = m.vip_id WHERE m.agent_id = $1 -- Active VIP first (an agent has at most one). With NO active VIP, pick the most @@ -2367,6 +2374,14 @@ async def get_agent_keepalived_config(agent_name: str, x_api_key: Optional[str] "config_content": row['applied_config_content'], "config_hash": row['applied_config_hash'], "check_script": check_script, + # v1.10.4 adoption handoff. The agent refuses to overwrite a keepalived.conf + # without our ownership marker — the guard that protects a hand-maintained + # setup. Adoption does not weaken it: it authorises exactly ONE takeover, of + # exactly the file we analysed, by pinning its hash. If the file changed since + # adoption the hashes differ and the agent keeps refusing, so an edit made + # between adoption and Apply can never be silently overwritten. + "allow_takeover": bool(row['takeover_expected_hash']), + "takeover_expected_hash": row['takeover_expected_hash'], }, } except HTTPException: @@ -2431,6 +2446,100 @@ async def agent_keepalived_status(agent_name: str, status_data: dict, x_api_key: if conn: await close_database_connection(conn) + +@router.post("/{agent_name}/keepalived-discovery") +async def agent_keepalived_discovery(agent_name: str, payload: dict, x_api_key: Optional[str] = Header(None)): + """v1.10.4 — the agent reports a keepalived.conf it found on the node but does NOT own. + + This is what makes adopting a hand-maintained VIP possible: the heartbeat only carries the + VIP address and a best-effort MASTER/BACKUP, while rendering a node's config needs eleven + fields, so the file itself has to be read. Read-only on the agent side — reporting never + changes anything on the node. + + Auth mirrors /keepalived-status: a MISSING key is rejected outright, and because the token + is a shared install token a name mismatch is an advisory audit log rather than a 403. + + SECRETS: the reported content may contain the VRRP `auth_pass`. It is split immediately — + the password is Fernet-encrypted into its own column and the stored copy of the file has it + masked, so nothing readable through the API or a DB dump carries it in cleartext. The + parse result is never logged. + """ + conn = None + try: + from auth_middleware import validate_agent_api_key + agent_auth = await validate_agent_api_key(x_api_key) + if not x_api_key or not agent_auth: + raise HTTPException(status_code=401, detail="Invalid API key") + if agent_auth['name'] != agent_name: + logger.info(f"Agent '{agent_name}' reporting keepalived discovery using API key " + f"from agent '{agent_auth['name']}'") + + conn = await get_database_connection() + agent = await conn.fetchrow("SELECT id FROM agents WHERE name = $1", agent_name) + if not agent: + raise HTTPException(status_code=404, detail=f"Agent '{agent_name}' not found") + + config_path = (payload.get("config_path") or "/etc/keepalived/keepalived.conf")[:500] + exists = bool(payload.get("exists")) + if not exists: + # The file is gone (keepalived removed, or we adopted and now own it) — drop the row + # so the UI stops offering a stale candidate. + await conn.execute("DELETE FROM vip_discoveries WHERE agent_id = $1", agent['id']) + return {"status": "cleared"} + + content = payload.get("config_content") or "" + if len(content) > 256_000: + raise HTTPException(status_code=413, detail="keepalived.conf too large to analyse") + is_managed = bool(payload.get("is_managed")) + config_hash = hashlib.md5(content.encode("utf-8", "replace")).hexdigest() + + from services.keepalived_parser import analyse_keepalived_conf, KeepalivedParseError + from services.keepalived_config import encrypt_vrrp_secret + + parse_error = None + analysis = None + auth_enc = None + try: + analysis = analyse_keepalived_conf(content) + # Split the secret out of everything we persist or serve. + for cand in analysis.get("candidates", []): + secret = (cand.get("vip") or {}).pop("auth_pass", None) + cand["vip"]["has_auth_pass"] = bool(secret) + if secret and auth_enc is None: + auth_enc = encrypt_vrrp_secret(secret) + except KeepalivedParseError as exc: + parse_error = str(exc)[:500] + except Exception as exc: # noqa: BLE001 — a malformed file must not 500 the agent loop + parse_error = f"could not analyse the config ({type(exc).__name__})" + + masked = _AUTH_PASS_MASK_RE.sub(r"\1********", content) + await conn.execute(""" + INSERT INTO vip_discoveries + (agent_id, config_path, config_hash, is_managed, raw_config_masked, + auth_pass_encrypted, analysis, parse_error, reported_at) + VALUES ($1,$2,$3,$4,$5,$6,$7,$8,CURRENT_TIMESTAMP) + ON CONFLICT (agent_id) DO UPDATE SET + config_path = EXCLUDED.config_path, + config_hash = EXCLUDED.config_hash, + is_managed = EXCLUDED.is_managed, + raw_config_masked = EXCLUDED.raw_config_masked, + auth_pass_encrypted = EXCLUDED.auth_pass_encrypted, + analysis = EXCLUDED.analysis, + parse_error = EXCLUDED.parse_error, + reported_at = CURRENT_TIMESTAMP + """, agent['id'], config_path, config_hash, is_managed, masked, auth_enc, + json.dumps(analysis) if analysis is not None else None, parse_error) + return {"status": "recorded", "config_hash": config_hash} + except HTTPException: + raise + except Exception as e: + logger.error(f"keepalived-discovery failed for '{agent_name}': {e}") + raise HTTPException(status_code=500, detail="keepalived-discovery failed") + finally: + if conn: + await close_database_connection(conn) + + @router.get("/script-version") async def get_latest_script_version(platform: str = "macos"): """Get the latest available agent script version for specified platform""" From d92a7e96605dc540206eb835f4f1dcec0588ef42 Mon Sep 17 00:00:00 2001 From: "mustafa.ulukaya" Date: Tue, 11 Aug 2026 01:35:59 +0300 Subject: [PATCH 3/8] feat(vip): adopt a discovered keepalived instance into a managed VIP GET /api/vip/discoveries lists what the agents found; POST /api/vip/adopt turns one vrrp_instance into a managed VIP using the values from the node's own file instead of retyping them. The VIP is created PENDING like any other, so nothing reaches the node until it is applied from Apply Management. Adoption replaces the operator's file with our render, so the gate is the feature. Blockers fall into three kinds and only two are resolvable: - a LOSS ("our renderer cannot reproduce this, so adopting would delete it") can be accepted explicitly - that is an informed choice about a notify hook or an LVS section; - an UNKNOWN prefix length can be supplied, because picking a netmask for a live VIP would change its routing; - anything else is an IMPOSSIBILITY, not a loss: an absent virtual_router_id, a fractional advert_int, an unsupported auth_type. No flag waves those through. That rule now lives in one place, remaining_blockers(), so the endpoint and the UI cannot drift apart - and it is unit-testable, which matters because getting it wrong destroys a working config. Adoption keeps the VRRP identity it found: unlike create_vip, which allocates the next free VRID, a VRID already used in the pool is a hard 409. Silently renumbering would put the adopted node in a different VRRP domain from the peers that still run the original config. The member row records the reporting node's own role, priority and interface, and carries the one-shot takeover hash. The response returns the instance's unicast peers, because those nodes hold their own keepalived.conf and have to be adopted or added as members before the render describes a complete group. --- backend/routers/vip.py | 215 ++++++++++++++++++++++++++ backend/services/keepalived_parser.py | 33 ++++ 2 files changed, 248 insertions(+) diff --git a/backend/routers/vip.py b/backend/routers/vip.py index 223e52f..f7d51d4 100644 --- a/backend/routers/vip.py +++ b/backend/routers/vip.py @@ -985,3 +985,218 @@ async def vip_status(vip_id: int, authorization: str = Header(None)): # endpoint (cluster.py get_config_version_diff, vip-* branch) via render_vip_config_masked # above — there is no bespoke VIP preview endpoint, so VIP changes use the product's # standard "View Change" like every other entity (issue #27 follow-up). + + +# --------------------------------------------------------------------------- +# v1.10.4 — Adoption of a keepalived setup that already exists on the nodes +# --------------------------------------------------------------------------- +# The HA/VIP page starts empty on a fleet that already runs keepalived, because the flow is +# one-way: VIPs are declared here and pushed to the node, and nothing read what was already +# there. The agent now reports the keepalived.conf it found (read-only) into vip_discoveries; +# these two endpoints list those findings and turn one into a managed VIP. +# +# Adoption REPLACES the operator's file with our render, so it is gated hard: the parser +# reports every directive we cannot reproduce and every value we cannot know, and adoption +# refuses while any remain. See services/keepalived_parser.py for the reasoning. + + +def _discovery_row_to_api(row) -> dict: + """Shape a vip_discoveries row for the UI. Never includes the VRRP password: the stored + config copy is masked and the analysis has the secret replaced by a boolean.""" + analysis = row["analysis"] + if isinstance(analysis, str): + try: + analysis = json.loads(analysis) + except (json.JSONDecodeError, TypeError): + analysis = None + return { + "agent_id": row["agent_id"], + "agent_name": row["agent_name"], + "pool_id": row["pool_id"], + "pool_name": row["pool_name"], + "config_path": row["config_path"], + "config_hash": row["config_hash"], + "is_managed": row["is_managed"], + "parse_error": row["parse_error"], + "adopted_vip_id": row["adopted_vip_id"], + "reported_at": row["reported_at"].isoformat() if row["reported_at"] else None, + "config_preview": row["raw_config_masked"], + "analysis": analysis, + } + + +@router.get("/discoveries") +async def list_vip_discoveries(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. + """ + await _require(authorization, "read") + conn = await get_database_connection() + try: + try: + rows = await conn.fetch(""" + SELECT d.*, a.name AS agent_name, a.pool_id, p.name AS pool_name + 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 + ORDER BY d.reported_at DESC, d.id DESC + """) + except Exception as exc: # noqa: BLE001 — a missing relation degrades to empty (B-7) + logger.debug(f"vip_discoveries unavailable: {exc}") + return {"discoveries": []} + return {"discoveries": [_discovery_row_to_api(r) for r in rows]} + finally: + await close_database_connection(conn) + + +def _find_candidate(analysis: Optional[dict], instance_name: str) -> Optional[dict]: + for cand in ((analysis or {}).get("candidates") or []): + if cand.get("instance_name") == instance_name: + return cand + return None + + +@router.post("/adopt") +async def adopt_vip(payload: dict, request: Request, authorization: str = Header(None)): + """Turn one discovered vrrp_instance into a managed VIP. + + Body: agent_id, instance_name, name, [description], [prefix_length], [accept_data_loss]. + + Refuses while the parser reports blockers. Two of them are resolvable by the operator + rather than fatal: + * a missing prefix length can be supplied as `prefix_length` (we never guess a netmask + for a live VIP); + * "we would delete this directive" can be accepted with `accept_data_loss: true`, which + is an explicit choice to lose e.g. a notify hook. Everything else — an unknown VRID, a + fractional advert_int, an unsupported auth_type — is not a loss but an impossibility, + and no flag overrides it. + + The VIP is created PENDING like any other, so nothing reaches the node until the operator + applies it from Apply Management. + """ + current_user = await _require(authorization, "create") + agent_id = payload.get("agent_id") + instance_name = (payload.get("instance_name") or "").strip() + name = (payload.get("name") or "").strip() + if not agent_id or not instance_name or not name: + raise HTTPException(status_code=400, detail="agent_id, instance_name and name are required") + + conn = await get_database_connection() + try: + disc = await conn.fetchrow(""" + SELECT d.*, a.name AS agent_name, a.pool_id + FROM vip_discoveries d JOIN agents a ON a.id = d.agent_id + WHERE d.agent_id = $1 + """, int(agent_id)) + if not disc: + raise HTTPException(status_code=404, detail="No discovered keepalived config for that agent") + if disc["adopted_vip_id"]: + raise HTTPException(status_code=409, detail="This discovery has already been adopted") + if not disc["pool_id"]: + raise HTTPException(status_code=400, detail="The agent is not in a pool; assign it first") + if disc["parse_error"]: + raise HTTPException(status_code=422, + detail=f"Config could not be parsed: {disc['parse_error']}") + + analysis = disc["analysis"] + if isinstance(analysis, str): + analysis = json.loads(analysis) + cand = _find_candidate(analysis, instance_name) + if not cand: + raise HTTPException(status_code=404, + detail=f"No vrrp_instance '{instance_name}' in the report") + + vip_fields = dict(cand.get("vip") or {}) + member = dict(cand.get("member") or {}) + + # The operator may supply the one value we refuse to guess. + supplied_prefix = payload.get("prefix_length") + if vip_fields.get("prefix_length") is None and supplied_prefix is not None: + try: + vip_fields["prefix_length"] = int(supplied_prefix) + except (TypeError, ValueError): + raise HTTPException(status_code=400, detail="prefix_length must be an integer") + + # One source of truth for which blockers an operator may resolve (see the docstring on + # remaining_blockers): a supplied prefix, and an explicit acceptance of directives our + # renderer would delete. Nothing else is waivable. + from services.keepalived_parser import remaining_blockers + blockers = remaining_blockers( + list(cand.get("blockers") or []), + prefix_supplied=vip_fields.get("prefix_length") is not None, + accept_data_loss=bool(payload.get("accept_data_loss")), + ) + if blockers: + raise HTTPException(status_code=422, detail={ + "message": "This keepalived config cannot be adopted as-is", + "blockers": blockers, + }) + + vrid = vip_fields.get("virtual_router_id") + virtual_ip = vip_fields.get("virtual_ip") + if vrid is None or not virtual_ip or not member.get("network_interface"): + raise HTTPException(status_code=422, + detail="Incomplete candidate (vrid/address/interface)") + + # Adoption must keep the VRRP identity it found. A different VRID would create a second + # VRRP domain on the wire, so a collision inside the pool is a hard conflict, never an + # auto-reallocation the way create_vip does it. + clash = await conn.fetchrow(""" + SELECT id, name FROM vip_instances + WHERE pool_id = $1 AND is_active = TRUE AND virtual_router_id = $2 + """, disc["pool_id"], int(vrid)) + if clash: + raise HTTPException(status_code=409, detail=( + f"VRID {vrid} is already used by VIP '{clash['name']}' in this pool; the adopted " + f"config must keep its VRID, so resolve the collision first")) + + async with conn.transaction(): + vip_id = await conn.fetchval(""" + INSERT INTO vip_instances + (name, description, pool_id, virtual_ip, prefix_length, virtual_router_id, + advert_int, auth_pass_encrypted, use_unicast, track_haproxy, + is_active, last_config_status, adopted_at, created_by) + VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,TRUE,'PENDING',CURRENT_TIMESTAMP,$11) + RETURNING id + """, name, payload.get("description") or f"Adopted from {disc['agent_name']}", + disc["pool_id"], virtual_ip, int(vip_fields["prefix_length"]), int(vrid), + int(vip_fields.get("advert_int") or 1), + disc["auth_pass_encrypted"], # already Fernet-encrypted at ingest + bool(vip_fields.get("use_unicast")), bool(vip_fields.get("track_haproxy")), + current_user["id"]) + # The reporting node becomes a member with the role/priority its own file declares, + # and carries the one-shot takeover authorisation pinned to the hash we analysed. + await conn.execute(""" + INSERT INTO vip_members + (vip_id, agent_id, network_interface, role, priority, takeover_expected_hash) + VALUES ($1,$2,$3,$4,$5,$6) + """, vip_id, int(agent_id), member["network_interface"], + member.get("role") or "BACKUP", int(member.get("priority") or 100), + disc["config_hash"]) + await conn.execute( + "UPDATE vip_discoveries SET adopted_vip_id = $2 WHERE agent_id = $1", + int(agent_id), vip_id) + + await _stage_vip_version(conn, vip_id, "adopt", current_user["id"]) + await log_user_activity( + user_id=current_user["id"], action="adopt", resource_type="vip", + resource_id=str(vip_id), + details={"name": name, "virtual_ip": virtual_ip, "vrid": vrid, + "adopted_from_agent": disc["agent_name"], + "accepted_data_loss": bool(payload.get("accept_data_loss"))}, + ip_address=_client_ip(request), user_agent=_user_agent(request)) + + peers = list(cand.get("peers") or []) + return { + "id": vip_id, + "message": ("VIP adopted (PENDING — review it in Apply Management, then apply to hand " + "the node's keepalived.conf over to OpenManager)"), + # Adoption covers the node that reported. Its peers hold their own keepalived.conf, + # so they must be added as members (or adopted from their own report) before the VIP + # renders a complete unicast group. + "peers_to_add": peers, + } + finally: + await close_database_connection(conn) diff --git a/backend/services/keepalived_parser.py b/backend/services/keepalived_parser.py index 3948415..aa28bf1 100644 --- a/backend/services/keepalived_parser.py +++ b/backend/services/keepalived_parser.py @@ -489,6 +489,39 @@ def build_adoption_candidate(parsed: Dict[str, Any], instance: Dict[str, Any]) - return candidate +# Substrings that identify the two blocker classes an operator is allowed to resolve. They are +# matched rather than typed because the blocker text is what the UI shows; keeping the marker in +# the sentence means the message and the rule cannot drift apart. +_LOSS_MARKER = "would delete it" +_PREFIX_MARKER = "no explicit prefix length" + + +def remaining_blockers(blockers: List[str], *, prefix_supplied: bool = False, + accept_data_loss: bool = False) -> List[str]: + """Blockers that survive what the operator is permitted to resolve. + + Exactly two classes are resolvable, and the distinction is the whole safety argument: + + * a missing prefix length is *unknown*, and the operator can supply it — we refuse to pick + a netmask for a live VIP ourselves; + * "our renderer cannot reproduce this, so adopting would delete it" is a *loss*, and losing + it can be an informed choice. + + Everything else — an unknown virtual_router_id, a fractional advert_int, an unsupported + auth_type, an address on a different interface — is neither unknown nor a loss but an + impossibility, and no flag may wave it through. This is the single source of truth for that + rule; the endpoint and the UI both derive from it. + """ + out: List[str] = [] + for b in blockers or []: + if prefix_supplied and _PREFIX_MARKER in b: + continue + if accept_data_loss and _LOSS_MARKER in b: + continue + out.append(b) + return out + + def analyse_keepalived_conf(text: str) -> Dict[str, Any]: """Parse + map in one call: the shape the discovery endpoint stores and the UI renders.""" parsed = parse_keepalived_conf(text) From 164841219aaf018a5ddf7c1ff5131b20bd9a64a5 Mon Sep 17 00:00:00 2001 From: "mustafa.ulukaya" Date: Tue, 11 Aug 2026 01:36:19 +0300 Subject: [PATCH 4/8] feat(agent): report an unmanaged keepalived.conf and honour a one-shot takeover Two additions to the Linux agent, both inside the existing keepalived converge function so no new poll or timer is introduced. Discovery is strictly read-only: when the node has a keepalived.conf without OpenManager's ownership marker, the agent posts it so an existing VIP can be adopted from the UI. Nothing is written to the node. It is rate-limited by content - the md5 of the last report is cached next to the config, so a file that may carry the VRRP password is posted only when it actually changes rather than every cycle. Once we own the file there is nothing left to adopt, so the record is cleared exactly once. The content is JSON-encoded with `jq -Rs` so newlines survive verbatim and the hash the server pins the takeover to is the hash of what is really on disk. The ownership guard now has exactly one exception, and it does not weaken it. Previously any file without the marker was refused, which is what protects a hand-maintained setup - and also what would block adoption forever. The server authorises a single takeover of a specific file by sending the md5 the operator adopted from, and the agent overwrites only when the on-disk hash still matches. If the file changed in between, the agent refuses again and reports why, so an edit made after adoption wins over the stale adoption instead of being destroyed. The fallback latest Linux agent version moves 2.0.0 -> 2.1.0 so nodes pull the new script through the normal upgrade path. Discovery simply does not happen on a node that has not upgraded yet. --- backend/utils/agent_scripts/linux_install.sh | 64 +++++++++++++++++++- 1 file changed, 61 insertions(+), 3 deletions(-) diff --git a/backend/utils/agent_scripts/linux_install.sh b/backend/utils/agent_scripts/linux_install.sh index 2b8240c..e69a713 100644 --- a/backend/utils/agent_scripts/linux_install.sh +++ b/backend/utils/agent_scripts/linux_install.sh @@ -1711,6 +1711,45 @@ fetch_and_deploy_keepalived_config() { chk="$(dirname "$conf")/check_haproxy.sh" if [[ -f "$conf" ]] && grep -q "$marker" "$conf" 2>/dev/null; then we_own="true"; fi + # 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 + # to the node. The heartbeat cannot carry this — it has the VIP address and a best-effort + # MASTER/BACKUP, while rendering a node's config needs eleven fields. + # + # Rate limited by content: the hash of the last report is cached next to the config, so the + # file (which may contain the VRRP password) is posted only when it actually changes, not on + # every cycle. Once we own the file there is nothing to adopt, so the record is cleared once. + _kp_discover() { + local cache="$(dirname "$conf")/.hom_discovery_hash" cur_hash="" body content_json + 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" \ + -H "X-API-Key: $AGENT_TOKEN" -H "Content-Type: application/json" \ + -d "{\"config_path\":\"$conf\",\"exists\":false}" >/dev/null 2>&1 || 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 + # 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" \ + -H "X-API-Key: $AGENT_TOKEN" -H "Content-Type: application/json" \ + --data-binary "$body" 2>/dev/null; then + printf '%s' "$cur_hash" > "$cache" 2>/dev/null + log "INFO" "KEEPALIVED: reported an unmanaged keepalived.conf for adoption" + fi + } + _kp_discover + _kp_report() { # $1=state $2=vip_id(or empty) $3=hash $4=message local vid="${2:-null}"; [[ -z "$2" ]] && vid="null" curl -k -s --connect-timeout 10 --max-time 30 -X POST "$MANAGEMENT_URL/api/agents/$AGENT_NAME/keepalived-status" \ @@ -1774,10 +1813,29 @@ fetch_and_deploy_keepalived_config() { [[ -z "$new_conf" ]] && return 0 # Ownership guard: never overwrite a keepalived.conf we don't own. + # + # v1.10.4 adoption is the ONE exception, and it does not weaken the guard: the server + # authorises a single takeover of a specific file by pinning the md5 the operator adopted + # from. We overwrite only when that hash still matches what is on disk, so a config edited + # between adoption and Apply is still refused — the operator's later edit wins over a stale + # adoption rather than being silently destroyed. if [[ -f "$conf" && "$we_own" != "true" ]]; then - log "WARN" "KEEPALIVED: $conf is externally managed — refusing to overwrite" - _kp_report "externally_managed" "$vip_id" "" "pre-existing unmanaged keepalived.conf" - return 0 + local allow_takeover expected_hash disk_hash + allow_takeover=$(echo "$resp" | jq -r '.keepalived.allow_takeover // false' 2>/dev/null) + expected_hash=$(echo "$resp" | jq -r '.keepalived.takeover_expected_hash // empty' 2>/dev/null) + disk_hash=$(md5sum "$conf" 2>/dev/null | awk '{print $1}') + if [[ "$allow_takeover" == "true" && -n "$expected_hash" && "$disk_hash" == "$expected_hash" ]]; then + log "INFO" "KEEPALIVED: adopting $conf (one-shot takeover authorised; on-disk hash matches)" + elif [[ "$allow_takeover" == "true" ]]; then + log "WARN" "KEEPALIVED: adoption authorised but $conf changed since it was adopted — refusing" + _kp_report "externally_managed" "$vip_id" "$disk_hash" \ + "config changed after adoption; re-adopt to pick up the current file" + return 0 + else + log "WARN" "KEEPALIVED: $conf is externally managed — refusing to overwrite" + _kp_report "externally_managed" "$vip_id" "" "pre-existing unmanaged keepalived.conf" + return 0 + fi fi # Hybrid install: install keepalived only if missing. From 1ca811e211f83849ba65c020c261f570d7b4042f Mon Sep 17 00:00:00 2001 From: "mustafa.ulukaya" Date: Tue, 11 Aug 2026 01:36:19 +0300 Subject: [PATCH 5/8] feat(ui): surface unmanaged keepalived on HA/VIP with an Adopt flow The page came up empty on a fleet that already runs keepalived, with nothing to explain why. It now lists the nodes whose keepalived.conf the agent found and deliberately left alone, in a section separate from managed VIPs so the distinction is visible: OpenManager is not managing these. Each discovered vrrp_instance shows the address, VRID, and this node's own role, priority and interface, plus whether it can be adopted. Adopt opens a modal that states what will happen rather than just asking for confirmation: which directives would be deleted on takeover (with an explicit tick to accept that, disabled otherwise), which values were assumed from keepalived's documented defaults rather than read from the file, and the config itself with the VRRP password masked. Blockers are split the same way the backend splits them, from the same marker strings, so the button cannot offer an adoption the API would reject: a loss is waivable with the tick, a missing prefix is resolvable by supplying it, and an impossibility disables Adopt outright with the reason shown. After a successful adopt the peers of the adopted instance are named, because those nodes hold their own keepalived.conf and the VIP is not a complete VRRP group until they are members too. --- frontend/src/components/VIPManagement.js | 263 ++++++++++++++++++++++- 1 file changed, 260 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/VIPManagement.js b/frontend/src/components/VIPManagement.js index ee028eb..8b71483 100644 --- a/frontend/src/components/VIPManagement.js +++ b/frontend/src/components/VIPManagement.js @@ -1,7 +1,7 @@ import React, { useState, useEffect, useCallback } from 'react'; import { Table, Button, Space, Modal, Form, Input, InputNumber, Select, Tag, message, - Switch, Typography, Card, Alert, Tooltip, Spin + Switch, Typography, Card, Alert, Tooltip, Spin, Checkbox } from 'antd'; import { PlusOutlined, EditOutlined, DeleteOutlined, ReloadOutlined, WarningOutlined, @@ -46,6 +46,23 @@ const parseArr = (v) => { return []; }; +// v1.10.4 — adoption blockers come back as prose from the parser. Two classes are resolvable by +// the operator and the rest are not, so the modal has to tell them apart: +// * `loss` — "our renderer cannot reproduce this, so adopting would delete it". A deliberate +// choice, waivable with an explicit tick. +// * `prefix` — the address has no explicit prefix length. Supplying it resolves the blocker; +// we never guess a netmask for a live VIP. +// * `hard` — an unknown VRID, a fractional advert_int, an unsupported auth_type. Not losses +// but impossibilities; nothing in the UI may override them. +const splitBlockers = (blockers) => { + const list = blockers || []; + return { + loss: list.filter((b) => b.includes('would delete it')), + prefix: list.filter((b) => b.includes('no explicit prefix length')), + hard: list.filter((b) => !b.includes('would delete it') && !b.includes('no explicit prefix length')), + }; +}; + // This component uses raw fetch(), but extractApiError expects an axios-shaped error // (err.response.data). Read the fetch Response body and reuse the envelope-aware extractor // so backend messages — e.g. the 409 "node already in VIP X" — actually reach the user. @@ -67,6 +84,12 @@ const VIPManagement = () => { // Delete confirmation (with opt-in package uninstall) + diagnostics modal state. const [deleteTarget, setDeleteTarget] = useState(null); const [showL2Note, setShowL2Note] = useState(false); + // v1.10.4 — VIP adoption from what the agents found on their nodes. + const [discoveries, setDiscoveries] = useState([]); + const [adoptTarget, setAdoptTarget] = useState(null); // { discovery, candidate } + const [adoptAcceptLoss, setAdoptAcceptLoss] = useState(false); + const [adopting, setAdopting] = useState(false); + const [adoptForm] = Form.useForm(); const [diagVip, setDiagVip] = useState(null); const [diagData, setDiagData] = useState(null); const [diagLoading, setDiagLoading] = useState(false); @@ -98,11 +121,77 @@ const VIPManagement = () => { } }, []); + // 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() }); + 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); + } + }, []); + useEffect(() => { fetchVips(); - const t = setInterval(fetchVips, 30000); // live MASTER/BACKUP via existing detection pipeline + fetchDiscoveries(); + const t = setInterval(() => { fetchVips(); fetchDiscoveries(); }, 30000); // live MASTER/BACKUP via existing detection pipeline return () => clearInterval(t); - }, [fetchVips]); + }, [fetchVips, fetchDiscoveries]); + + const openAdopt = (discovery, candidate) => { + setAdoptTarget({ discovery, candidate }); + setAdoptAcceptLoss(false); + adoptForm.setFieldsValue({ + name: `${discovery.agent_name}-${candidate?.vip?.virtual_ip || 'vip'}`, + prefix_length: candidate?.vip?.prefix_length ?? undefined, + }); + }; + + const submitAdopt = async () => { + if (!adoptTarget) return; + let values; + try { values = await adoptForm.validateFields(); } catch { return; } + setAdopting(true); + try { + const res = await fetch('/api/vip/adopt', { + method: 'POST', + headers: authHeaders(), + body: JSON.stringify({ + agent_id: adoptTarget.discovery.agent_id, + instance_name: adoptTarget.candidate.instance_name, + name: values.name, + description: values.description || undefined, + prefix_length: values.prefix_length ?? undefined, + accept_data_loss: adoptAcceptLoss || undefined, + }), + }); + if (!res.ok) { + message.error(await fetchApiError(res, 'Adoption failed'), 8); + return; + } + const data = await res.json(); + message.success(data.message || 'VIP adopted', 8); + if ((data.peers_to_add || []).length > 0) { + // The adopted node's peers keep their own keepalived.conf, so the VIP is not a complete + // VRRP group until they are members too. + message.info( + `This instance has ${data.peers_to_add.length} unicast peer(s) (${data.peers_to_add.join(', ')}). ` + + 'Adopt or add those nodes as members before applying, or the rendered config will have no peers.', + 12); + } + setAdoptTarget(null); + fetchVips(); + fetchDiscoveries(); + } catch (e) { + message.error('Adoption failed'); + } finally { + setAdopting(false); + } + }; // Build the participating-nodes table from the pool's EXISTING agents (installed via the // standard Agent Management process). On edit, pre-select the VIP's current members. @@ -410,6 +499,174 @@ const VIPManagement = () => { + {/* v1.10.4 — keepalived that already exists on a node. Shown separately from managed VIPs + because OpenManager is NOT managing these: the agent found them, reported them, and + deliberately left them untouched. */} + {discoveries.length > 0 && ( + + + Unmanaged keepalived detected on {discoveries.length} node(s) + + }> + + The agent read each keepalived.conf and left it untouched — nothing + on these nodes has been changed. Adopting one creates a managed VIP from the values + in that file, and the node's config is only handed over when you apply it from + Apply Management. Adoption replaces the file with OpenManager's render, so anything + it cannot reproduce is listed as a blocker rather than silently dropped. + + } + /> +
`${r.agent_id}:${r.instance_name}`} + size="small" + pagination={false} + dataSource={discoveries.flatMap((d) => { + const cands = (d.analysis?.candidates || []); + if (cands.length === 0) { + return [{ agent_id: d.agent_id, discovery: d, instance_name: '—', candidate: null }]; + } + return cands.map((c) => ({ + agent_id: d.agent_id, discovery: d, instance_name: c.instance_name, candidate: c, + })); + })} + columns={[ + { title: 'Node', dataIndex: ['discovery', 'agent_name'], key: 'agent', + render: (_v, r) => ( + + {r.discovery.agent_name} + {r.discovery.pool_name || 'no pool'} + + ) }, + { title: 'Instance', dataIndex: 'instance_name', key: 'instance' }, + { title: 'Virtual IP', key: 'vip', + render: (_v, r) => (r.candidate?.vip?.virtual_ip + ? {r.candidate.vip.virtual_ip} + {r.candidate.vip.prefix_length != null ? `/${r.candidate.vip.prefix_length}` : ''} + : —) }, + { title: 'VRID', key: 'vrid', + render: (_v, r) => (r.candidate?.vip?.virtual_router_id ?? —) }, + { title: 'This node', key: 'member', + render: (_v, r) => (r.candidate ? ( + + + {r.candidate.member.role} + + + prio {r.candidate.member.priority} · {r.candidate.member.network_interface} + + + ) : —) }, + { title: 'Adoptable', key: 'adoptable', + render: (_v, r) => { + if (r.discovery.parse_error) { + return unparseable; + } + if (!r.candidate) return no vrrp_instance; + if (r.candidate.adoptable) return yes; + const { hard } = splitBlockers(r.candidate.blockers); + return ( + + + {hard.length ? `${hard.length} blocker(s)` : 'needs review'} + + + ); + } }, + { title: 'Actions', key: 'actions', + render: (_v, r) => ( + + ) }, + ]} + /> + + )} + + {/* Adopt modal — shows what will be taken over, what was assumed, and what would be lost. */} + setAdoptTarget(null)} + onOk={submitAdopt} + confirmLoading={adopting} + okText="Adopt as PENDING" + width={720} + okButtonProps={{ + disabled: !!adoptTarget && (() => { + const { loss, hard } = splitBlockers(adoptTarget.candidate.blockers); + return hard.length > 0 || (loss.length > 0 && !adoptAcceptLoss); + })(), + }} + > + {adoptTarget && (() => { + const cand = adoptTarget.candidate; + const { loss, prefix, hard } = splitBlockers(cand.blockers); + return ( + <> + {hard.length > 0 && ( + + {hard.map((b, i) =>
  • {b}
    • )} + } /> + )} + {loss.length > 0 && ( + +
      + {loss.map((b, i) =>
    • {b}
      • )} +
    + setAdoptAcceptLoss(e.target.checked)}> + I understand these will be lost when the config is handed over + + + } /> + )} + {(cand.defaulted || []).length > 0 && ( + + )} +
    + + + + {prefix.length > 0 && ( + + + + )} + + + + + + Config found at {adoptTarget.discovery.config_path} — the VRRP + password is masked below and is carried over encrypted. + + 
    +                {adoptTarget.discovery.config_preview || '(not available)'}
+
    + + ); + })()} +     + Date: Tue, 11 Aug 2026 01:36:35 +0300 Subject: [PATCH 6/8] test(vip): cover the adoption gate and the VRRP password masking The gate decides whether an operator's working keepalived.conf gets replaced, so its rule is pinned directly: a supplied prefix resolves only the prefix blocker, accepting data loss resolves only the loss blocker, and setting both still cannot wave through an impossibility like an unknown VRID or an unsupported auth_type. The gate matches on substrings of the blocker prose the UI displays, which means a reworded message would silently stop being waivable. One test therefore feeds real parser output through it in both directions rather than hand-written strings, so the message and the rule are checked together. Also asserts that masking leaves no trace of a password containing spaces while keeping the rest of the config readable, using the router's own regex so the test breaks if it is ever loosened. --- backend/tests/test_keepalived_parser.py | 57 +++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/backend/tests/test_keepalived_parser.py b/backend/tests/test_keepalived_parser.py index 0b5d92d..487a8b1 100644 --- a/backend/tests/test_keepalived_parser.py +++ b/backend/tests/test_keepalived_parser.py @@ -282,3 +282,60 @@ def test_our_own_render_round_trips_with_zero_blockers(): assert cand2["adoptable"] is True, cand2["blockers"] assert cand2["vip"]["use_unicast"] is False and cand2["vip"]["track_haproxy"] is False assert cand2["vip"]["auth_pass"] is None + + +# --- v1.10.4 adoption gate: which blockers an operator may resolve -------------------------- + + +def test_only_prefix_and_data_loss_are_waivable(): + from services.keepalived_parser import remaining_blockers + + loss = "line 9: `notify_master \"/x.sh\"` — OpenManager's renderer cannot reproduce this, so adopting would delete it" + prefix = "`10.0.0.5` has no explicit prefix length; state it during adoption so the netmask cannot change on takeover" + hard_vrid = "no virtual_router_id — it cannot be guessed: a wrong VRID puts the nodes in separate VRRP domains" + hard_auth = "auth_type AH is not supported (only PASS is rendered)" + all_four = [loss, prefix, hard_vrid, hard_auth] + + # Nothing waived: everything survives. + assert remaining_blockers(all_four) == all_four + # A supplied prefix resolves ONLY the prefix blocker. + assert remaining_blockers(all_four, prefix_supplied=True) == [loss, hard_vrid, hard_auth] + # Accepting data loss resolves ONLY the loss blocker. + assert remaining_blockers(all_four, accept_data_loss=True) == [prefix, hard_vrid, hard_auth] + # Both together still cannot wave through an impossibility — this is the property that stops + # a UI flag from destroying a VIP whose VRID or auth_type we could not reproduce. + assert remaining_blockers(all_four, prefix_supplied=True, accept_data_loss=True) == \ + [hard_vrid, hard_auth] + # And an adoptable candidate stays adoptable. + assert remaining_blockers([]) == [] + + +def test_waiver_markers_match_the_messages_the_parser_actually_emits(): + # The gate matches on substrings of the blocker prose, so a reworded message would silently + # stop being waivable. Pin both directions against real parser output. + from services.keepalived_parser import remaining_blockers + + no_prefix = HANDWRITTEN.replace("10.0.0.100/24 dev eth0", "10.0.0.100 dev eth0") + blockers = _only_candidate(no_prefix)["blockers"] + assert blockers, "expected a prefix blocker" + assert remaining_blockers(blockers, prefix_supplied=True) == [] + + with_hook = HANDWRITTEN.replace( + " track_script {", ' notify_master "/usr/local/bin/promote.sh"\n track_script {') + blockers = _only_candidate(with_hook)["blockers"] + assert blockers, "expected a data-loss blocker" + assert remaining_blockers(blockers, accept_data_loss=True) == [] + + +def test_auth_pass_masking_leaves_no_trace_of_the_secret(): + # The discovered config is stored and served to the UI, so the ingest endpoint masks the VRRP + # password. Reuse the router's own regex so the test breaks if it is loosened. + from routers.agent import _AUTH_PASS_MASK_RE + + secret = "s3cr3t with spaces" + text = HANDWRITTEN.replace("auth_pass s3cr3t", f"auth_pass {secret}") + masked = _AUTH_PASS_MASK_RE.sub(r"\1********", text) + assert secret not in masked and "s3cr3t" not in masked + assert "auth_pass ********" in masked + # Everything else survives, so the preview is still useful. + assert "virtual_router_id 51" in masked and "10.0.0.100/24 dev eth0" in masked From 709817fec39af83d874c1225ab8c6bf40d9d5f22 Mon Sep 17 00:00:00 2001 From: "mustafa.ulukaya" Date: Tue, 11 Aug 2026 01:36:35 +0300 Subject: [PATCH 7/8] chore(version): bump to 1.10.4 - adopt an existing keepalived VIP --- backend/version.json | 4 ++-- frontend/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/version.json b/backend/version.json index ef98c45..35de075 100644 --- a/backend/version.json +++ b/backend/version.json @@ -1,5 +1,5 @@ { - "version": "1.10.3", - "releaseName": "Multi-account ACME — the wizard honours the selected account", + "version": "1.10.4", + "releaseName": "Adopt an existing keepalived VIP into HA/VIP management", "releaseDate": "2026-08-08" } diff --git a/frontend/package.json b/frontend/package.json index 2c1eec2..a05900a 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "haproxy-openmanager-frontend", - "version": "1.10.3", + "version": "1.10.4", "description": "HAProxy Load Balancer Management UI", "license": "AGPL-3.0-or-later", "dependencies": { From 78af849fdc58c6efda71055ab5e7b2a6de44d27f Mon Sep 17 00:00:00 2001 From: "mustafa.ulukaya" Date: Tue, 11 Aug 2026 01:36:35 +0300 Subject: [PATCH 8/8] docs(v1.10.4): document VIP adoption and its upgrade caveats Release note covering why the page was empty, why the heartbeat could not drive adoption, and how the blocker gate decides what may and may not be waived. The upgrade notes lead with the two things an operator has to act on rather than burying them. This release bumps SCHEMA_VERSION, which re-seeds the four built-in roles - the first re-seed since v1.9.0, because the three releases in between did not bump it - so role customizations have to be re-applied. And the Linux agent script changed, so discovery does not start until nodes pull it; until then a node simply never appears in the list. Also states the parts that are easy to get wrong: nothing is taken over implicitly, adoption can refuse on purpose and why, a multi-node VIP needs every peer adopted before applying, where the VRRP password lives, and what actually happens on a downgrade (the table goes unread, an adopted-but-unapplied VIP loses its takeover authorisation and the node keeps its original config). --- README.md | 1 + UPGRADE_GUIDE.md | 49 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/README.md b/README.md index 449ac3e..a4dadb3 100644 --- a/README.md +++ b/README.md @@ -2474,6 +2474,7 @@ Developed with ❤️ for the HAProxy community ## Release Notes +- **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. - **v1.10.2** (2026-08-08) — **Dark mode fixes on Apply Management**: several panels on the Apply Management page were painted with light-mode colour literals, so in dark mode the **Pending Changes** box rendered as a cream panel with light text on it — measured contrast **1.03:1**, effectively unreadable, now **11.50:1**. The same bug affected the added/removed rows in the *View Change* diff (2.21:1 and 2.99:1, now 5.49:1 and 4.01:1), the ACME and pending-version panels, the VIP pending-delete row, and the agent-error recommendation box; all now derive from theme tokens. Separately, **static confirm dialogs came up white in dark mode**: in Ant Design 5 the static `Modal.confirm` / `message` / `notification` APIs render into their own detached root and never see the app's `ConfigProvider`, so they always used the light algorithm. Registering `ConfigProvider.config({ holderRender })` once at the app root fixes **every** static dialog in the application (12 components use them), not only this page. Light mode is byte-identical — each token resolves under the default algorithm to exactly the literal it replaced. Frontend only: no schema, API, environment or agent change. - **v1.10.1** (2026-08-08) — **CSR private key encrypted at rest** (Issue #53): the private key of a **pending** CSR is now Fernet-encrypted in the database instead of stored as PEM. It is the one key in the system worth protecting this way — it sits idle for the entire signing window (days to weeks), is never transmitted to an agent, and is destroyed the moment the signed certificate is imported; `ssl_certificates.private_key_content` and the ACME order keys are unchanged, because agents must receive those in plaintext on every poll. The token replaces the PEM in the **same column**, so there is **no schema change and no `SCHEMA_VERSION` bump** (and therefore no re-seed of the built-in roles). CSRs created before this release keep a raw PEM and are still read transparently, so anything already out for signature imports normally with no data migration. The key derives from `SECRET_KEY` via HKDF with its own info string, independent of the VIP/MFA/DNS keys, and an optional `CSR_ENCRYPTION_KEY` enables independent rotation — rotating `SECRET_KEY` without it makes pending CSR keys unrecoverable, which now fails with an explicit "delete and re-create this CSR" error rather than a misleading key-mismatch. `.env.template` now documents all four per-purpose encryption keys. No API, UI or agent change. diff --git a/UPGRADE_GUIDE.md b/UPGRADE_GUIDE.md index c3a1a95..c8778cf 100644 --- a/UPGRADE_GUIDE.md +++ b/UPGRADE_GUIDE.md @@ -1,3 +1,52 @@ +# Upgrade Notes — v1.10.4 (Adopt an existing keepalived VIP) + +**Additive, but this release DOES bump the schema — read the role warning below.** Nothing on any +node changes until you adopt a VIP and apply it. + +- **Schema:** `SCHEMA_VERSION` bumps to `11`, so on first start the (idempotent) migration + sequence re-runs once and adds **one new table** (`vip_discoveries`) plus two additive columns + (`vip_instances.adopted_at`, `vip_members.takeover_expected_hash`). **No existing table is + altered**, no existing row changes, and the admin password is not reset. +- **⚠️ Built-in roles are re-seeded to their defaults** — the pre-existing behaviour of every + `SCHEMA_VERSION` bump. If you customised `super_admin` / `operator` / `security_admin` / + `viewer`, **re-apply those changes after upgrading**. (The three previous releases did not bump + the version, so this is the first re-seed since v1.9.0.) No new permission strings are + introduced: discovery and adoption are governed by the existing `vip.read` / `vip.create`. +- **⚠️ The Linux agent script changed, and discovery does not start until nodes run it.** The + fallback latest Linux agent version moves `2.0.0` → `2.1.0`, so nodes will pull the new script + through the normal agent-upgrade path. The addition is **read-only**: the agent reads the + `keepalived.conf` it does not own and reports it, rate-limited to once per content change. It + writes nothing new to the node. Until a node has upgraded, it simply never appears under + *Unmanaged keepalived detected*. +- **Nothing is taken over implicitly.** The agent still refuses to overwrite a `keepalived.conf` + that lacks OpenManager's ownership marker. Adoption authorises exactly **one** takeover of + exactly the file that was analysed, pinned to its md5: if the file changes between adoption and + Apply, the agent refuses again and reports `externally_managed` rather than clobbering your + edit. Re-adopt to pick up the current file. +- **Adoption can refuse, on purpose.** It replaces the file with OpenManager's render, so anything + the renderer cannot reproduce would be destroyed. Those directives are listed as blockers — + `notify_*` failover hooks, `vrrp_sync_group`, LVS `virtual_server` sections, a second address in + one instance, a custom `track_script`, extra `global_defs`. You can accept that loss explicitly + with a tick, but a value that is *unknown* rather than lost (an absent `virtual_router_id`, or + an address with no prefix length) cannot be waived — the VRID is fatal to guess and the prefix + has to be supplied, because picking a netmask for a live VIP would change its routing. +- **Multi-node VIPs need every node.** Adoption covers the node that reported. Its unicast peers + hold their own `keepalived.conf`, so adopt or add them as members before applying — otherwise + the render has no peers. The UI says so after a successful adopt. +- **Secrets:** the reported config may contain the VRRP `auth_pass`. It is split at ingest — the + password is Fernet-encrypted into its own column (same key path as `vip_instances`, + `VIP_ENCRYPTION_KEY` falling back to a key derived from `SECRET_KEY`) and the stored copy of the + file has it masked, so nothing readable through the API, the UI preview or a DB dump carries it + in cleartext. +- **Rollback:** downgrading to 1.10.3 leaves `vip_discoveries` as an unused table and the two new + columns unread; managed VIPs keep working. One caveat: a VIP adopted on 1.10.4 but **not yet + applied** loses its takeover authorisation on downgrade, so the node's original config stays in + place and the VIP sits PENDING — harmless, but re-adopt after upgrading again. Agents already on + script 2.1.0 keep reporting discoveries to an endpoint that no longer exists; the report fails + quietly and nothing on the node is affected. + +--- + # Upgrade Notes — v1.10.3 (Multi-account ACME wizard fix) **Frontend only. Nothing to do on upgrade.** No schema, no `SCHEMA_VERSION` bump, no API change, no