diff --git a/README.md b/README.md index 00b2571..15e0ddf 100644 --- a/README.md +++ b/README.md @@ -2475,6 +2475,7 @@ Developed with ❤️ for the HAProxy community ## Release Notes +- **v1.11.3** (2026-10-16) — **Config Import keeps `stick-table` and `stick` rules instead of dropping them**: the import parser had no branch for `stick-table` (frontend or backend) or for backend `stick on|match|store-request|store-response`, so those lines were discarded **without a warning**. A frontend that kept its `track-sc0` / `sc_*` counter rules then received the renderer's fallback table (`type ip size 100k expire 30s store http_req_rate(10s)`), so a re-defined table such as `size 200k expire 3h store http_req_rate(10s),gpc0_rate(10s),gpt0` was silently replaced by the default while the `gpc0` rules imported alongside it referenced counters the table did not store. These lines are now imported as-is into the frontend/backend **Options** field (no schema change) with an import warning, and an import that carries only a stick-table keeps the existing option lines. The table the platform generates itself for Rate Limit / WAF `rate_limit` rules is recognised and not stored, so a round-trip of a generated config shows no change and later rule edits still take effect. The import preview also deleted every `http-request track-sc0 src` / `sc_http_req_rate(0)` line as "auto-generated"; it now does so only where the platform regenerates them (frontend Rate Limit or a WAF `rate_limit` rule), so operator trackers on other frontends survive. Backends now render a single `stick-table` (the first; a second one is a fatal HAProxy error) and the preview warns about a table that does not store `http_req_rate` while Rate Limit is active, a duplicate table left in Request Headers, `stick on` without a table, and `peers` references. Configs without stick lines import and render byte-for-byte as before. No schema change, no agent change. - **v1.11.2** (2026-10-01) — **Read-only roles read SSL certificates, and private keys need `ssl.download`**: the UI never fills in `user_pool_access`, so every non-admin user created there has no pool access, and the SSL list (`?cluster_id=`) and SSL detail answered **403** on read while clusters, frontends, backends, WAF rules, agents and VIPs were readable. A role that grants `ssl.read` (or `waf.read` for WAF stats) now reads that resource on every cluster; without it the pool check still applies, and write endpoints are unchanged. Separately, `GET /api/ssl/certificates/{id}` and the SSL diff in Apply Management returned the **private key to any authenticated user**, including `viewer`. The key is now included only with `ssl.download` plus pool access; everyone else gets `private_key_content: null` with `private_key_redacted: true`, and the UI shows the key as hidden while editing keeps the stored key. Custom roles with `ssl.update` but no `ssl.download` lose sight of the key. No schema change, no agent change. - **v1.11.1** (2026-08-15) — **A node can no longer be hidden from adoption for good, and Config Import reaches a freshly installed agent**: the agent posts an unmanaged `keepalived.conf` for adoption and caches the hash of what it sent, so the file (which carries the VRRP password) is re-posted only when it changes. Delivery was judged by `curl`'s exit code, which is **0 for 5xx as well**, so a report the server *rejected* was recorded as delivered — and because a hand-maintained config does not change on its own, that node dropped out of *Unmanaged keepalived detected* permanently, curable only by deleting a cache file on the node by hand. Now: the report is cached only on a 2xx; `GET /agents/{name}/keepalived-config` tells the agent whether the server actually holds a discovery for it, so nodes stuck from earlier releases recover by themselves on the next poll; a **400/413/422** records the refusal so identical bytes are not re-posted forever (4xx and 5xx are never sampled out of the request log, so an unattended loop would write a row carrying the whole config every cycle), while **401 and 404 keep retrying** because in this system they mean a token rotation or an agent row briefly absent, not a bad payload. The same exit-code mistake in the *clear* path is fixed too, where it left a managed node still being offered for adoption. Separately, **Config Import silently did nothing on any agent that had never self-upgraded**: `check_config_requests` was defined in the installer and in the self-upgrade daemon but not in the body a fresh install writes, and its call site is guarded by `type`, so the operator asked a node for its `haproxy.cfg` and nothing arrived, with no error anywhere. And the cluster whose `keepalived_config_path` is handed to an agent is now resolved deterministically — a pool may hold several clusters, and the unordered join could return a different one between polls, pointing the agent at a file that does not exist. Agent-script change: sync the script from Agent Management and let the agents upgrade. No schema change. - **v1.11.0** (2026-08-14) — **Unified request/response log with configurable retention**: until now the only record of what happened was `user_activity_logs`, which stores non-GET **2xx** operations with no bodies — so when something failed you could see *that* the count went up, never *what was sent or what came back*. This release adds one queryable timeline covering **both directions**: every inbound API call (**including GETs and including 4xx/5xx**) with the user, client IP, status, duration and — redacted and size-capped — the request and response bodies; and every **outbound** HTTP call the backend makes, tagged with who it went to (ACME/Let's Encrypt, Cloudflare, GoDaddy, HAProxy stats, agents, the ACME diagnostics probe). Outbound rows **inherit the inbound request's id**, so one operator action and the CA/DNS calls it triggered read as a single trace — opening a failed *Request Certificate* shows the exact `POST /acme/new-order` and the CA's `429` body underneath it. Capture is a **pure-ASGI middleware that tees** the request and response streams rather than draining them, so no downstream handler is affected (notably the raw-body agent heartbeat), and rows are written by a **batching background writer** with a bounded queue, so the request path never waits on the database and a saturated logger drops rows visibly instead of blocking. Secrets never land: headers are an allowlist (`Authorization`/`Cookie` reduced to a presence marker), body keys and value shapes are redacted (passwords, tokens, API keys, private-key PEMs, JWTs), the **ACME JWS request body is never stored** (a stored `protected`+`signature` pair is a replayable credential — a summary is logged instead), DNS-provider errors record only the exception **type**, and the ACME HTTP-01 challenge endpoint is excluded so `key_authorization` is never captured. **Retention is operator-configurable** in *Settings → Request Log*: separate day counts for successful and failed rows (defaults 7 and 30) plus a hard row cap (500 000), whichever is reached first, pruned in **batches** under a Postgres advisory lock so a multi-million-row table cannot time out the delete or have every replica scan it at once. New **Request Log** page (`requestlog.read`) and retention/purge permission (`requestlog.manage`); `super_admin` and `security_admin` get both, `operator` gets read, `viewer` gets neither. **Successful agent polls are not logged** (`capture_agent_success`, default off; failures always are), which is what keeps the table's size a function of operator activity rather than of node count: measured at 2 424 bytes/row, a 200-node fleet would otherwise write 2.0M rows/day and reach the row cap in six hours, silently reducing the configured 7-day/30-day retention to a few hours for everything in the table. Cost is measured, not estimated: 27.7 µs per request on the hot path, 18.8 µs per row on the writer task, **0.096 % of one core at 500 nodes**. Adds one new table (`request_logs`) and its settings seed — SCHEMA_VERSION 11 → 12 (not 11: that number was taken by v1.10.4 while this was in review, and the version gate would have skipped the migration entirely on every existing install), auto-migrated, no existing table altered, no agent or rendered-config change. Kill switches: `REQUEST_LOG_ENABLED=false` (environment — the middleware is then never registered and costs nothing) or the `enabled` toggle in Settings (no restart). diff --git a/UPGRADE_GUIDE.md b/UPGRADE_GUIDE.md index 60bbf6d..fb731b2 100644 --- a/UPGRADE_GUIDE.md +++ b/UPGRADE_GUIDE.md @@ -1,3 +1,38 @@ +# Upgrade Notes — v1.11.3 (Config Import keeps `stick-table` and `stick` rules) + +**Backend change, no schema change, no agent change.** No `SCHEMA_VERSION` bump and no agent +upgrade is needed. + +- **`stick-table` is no longer dropped on Config Import.** Frontend `stick-table` and backend + `stick-table` / `stick on|match|store-request|store-response` lines are imported as-is into the + entity's **Options** field and listed as import warnings. Before this release they were discarded + silently, and a frontend that still used `track-sc` / `sc_*` counters rendered the default table + `stick-table type ip size 100k expire 30s store http_req_rate(10s)` instead of the imported one. +- **Re-import affected frontends and backends.** Entities imported with a custom `stick-table` on an + earlier version do not hold it; import the section again (the full frontend/backend block) to + store it. If you worked around the bug by putting a `stick-table` into Request Headers, remove it + from there after the re-import — the preview warns when both are present. +- **Options are merged by kind.** An import that carries only stick lines keeps the existing option + lines, and an import that carries only option lines keeps the existing stick lines. An import + carrying both replaces both, as before. +- **Generated lines are recognised, not stored.** The `stick-table type ip size 100k expire s + store http_req_rate(10s)` the platform renders for Rate Limit / WAF `rate_limit` rules, and the + matching `track-sc0 src` / `sc_http_req_rate(0)` rules, are skipped (with an ℹ️ note) only where + the platform regenerates them. A `track-sc0` tracker on a frontend without Rate Limit or a WAF + `rate_limit` rule is now kept; earlier versions removed it from the preview. +- **One stick-table per backend.** HAProxy rejects a second `stick-table` in the same proxy as a + fatal error; backends now render the first one and log the skipped duplicate, as frontends already + did. +- **`peers`:** a stick-table that references `peers ` needs that section in each node's + `haproxy.cfg` above the first `defaults`/`frontend`/`backend`/`listen` section, which the agent + preserves. The platform does not manage `peers` sections. +- Configs without stick lines parse, import and render exactly as before. + +**Rollback:** safe. No schema change. After a rollback the stick lines stay in Options and are still +rendered (Options is free-form); only the import behaviour returns to dropping them. + +--- + # Upgrade Notes — v1.11.2 (read-only roles read SSL; private keys need `ssl.download`) **Backend and frontend change, no schema change, no agent change.** No `SCHEMA_VERSION` bump, so the diff --git a/backend/routers/config.py b/backend/routers/config.py index 48010b8..a2d7897 100644 --- a/backend/routers/config.py +++ b/backend/routers/config.py @@ -17,6 +17,13 @@ from utils.config_templates import ( generate_config_from_template, TemplateCategory ) from utils.haproxy_config_parser import parse_haproxy_config +from utils.stick_table_import import ( + drop_platform_stick_tables, + frontend_uses_counter, + merge_options, + stick_advisories, + strip_auto_headers, +) from utils.logging_config import log_with_correlation, PerformanceLogger from auth_middleware import get_current_user_from_token, require_authenticated_user from database.connection import get_database_connection, close_database_connection @@ -978,26 +985,61 @@ async def parse_bulk_config( # field, WAF rules table) that the parser cannot distinguish from user-defined # configuration. Strip all auto-managed patterns so the preview and comparison # only reflect user-defined configuration. - def _is_auto_header(line): - s = line.strip() - if "is_acme_challenge" in s: - return True - if "track-sc0 src" in s: - return True - if "sc_http_req_rate(0)" in s: - return True - if s.startswith("http-request") and " waf_" in s: - return True - return False - - def _strip_auto_headers(headers_str): - if not headers_str: - return None - lines = [l for l in headers_str.split("\n") if not _is_auto_header(l)] - return "\n".join(lines) if lines else None + # + # Issue #72: the rate-limit lines (track-sc0 src / sc_http_req_rate(0)) are + # generator-owned only when the generator will emit them again for this + # frontend: its rate_limit is set or an active rate_limit WAF rule applies + # (same rule selection as services/haproxy_config.py). Otherwise they are + # the operator's own and were silently deleted on update. Errors fall back + # to "regenerates", i.e. the previous always-strip behaviour. + existing_frontend_rows = {} + platform_regenerates = {} + try: + global_rate_limit_waf = bool(await conn.fetchval(""" + SELECT EXISTS ( + SELECT 1 FROM waf_rules w + WHERE w.is_active = TRUE AND w.rule_type = 'rate_limit' + AND (w.cluster_id = $1 OR w.cluster_id IS NULL) + AND NOT EXISTS ( + SELECT 1 FROM frontend_waf_rules fwr WHERE fwr.waf_rule_id = w.id + ) + ) + """, request.cluster_id)) + except Exception as _waf_err: + logger.warning(f"BULK IMPORT: global rate_limit WAF lookup failed, assuming present: {_waf_err}") + global_rate_limit_waf = True + for frontend in frontends_data: + existing_frontend_rows[frontend["name"]] = await conn.fetchrow(""" + SELECT * FROM frontends + WHERE name = $1 AND cluster_id = $2 + ORDER BY is_active DESC + """, frontend["name"], request.cluster_id) + existing_frontend_ids = [r["id"] for r in existing_frontend_rows.values() if r] + frontends_with_rate_limit_waf = set() + if existing_frontend_ids and not global_rate_limit_waf: + try: + rows = await conn.fetch(""" + SELECT DISTINCT fw.frontend_id + FROM waf_rules w + JOIN frontend_waf_rules fw ON w.id = fw.waf_rule_id + WHERE w.is_active = TRUE AND w.rule_type = 'rate_limit' + AND fw.frontend_id = ANY($1::int[]) + """, existing_frontend_ids) + frontends_with_rate_limit_waf = {r["frontend_id"] for r in rows} + except Exception as _waf_err: + logger.warning(f"BULK IMPORT: assigned rate_limit WAF lookup failed, assuming present: {_waf_err}") + frontends_with_rate_limit_waf = set(existing_frontend_ids) + for frontend in frontends_data: + row = existing_frontend_rows[frontend["name"]] + platform_regenerates[frontend["name"]] = bool( + global_rate_limit_waf + or (row and row["rate_limit"]) + or (row and row["id"] in frontends_with_rate_limit_waf) + ) backends_data = [b for b in backends_data if b["name"] != "_acme_challenge_backend"] + stick_policy_warnings = [] for frontend in frontends_data: frontend["acl_rules"] = [ r for r in (frontend.get("acl_rules") or []) @@ -1007,7 +1049,9 @@ async def parse_bulk_config( r for r in (frontend.get("use_backend_rules") or []) if "_acme_challenge_backend" not in r ] - frontend["request_headers"] = _strip_auto_headers(frontend.get("request_headers")) + fe_regenerates = platform_regenerates[frontend["name"]] + frontend["request_headers"] = strip_auto_headers( + frontend.get("request_headers"), fe_regenerates) if frontend.get("tcp_request_rules"): lines = [ l for l in frontend["tcp_request_rules"].split("\n") @@ -1022,6 +1066,31 @@ async def parse_bulk_config( else: frontend["default_backend"] = None + # Issue #72: a generator-shaped stick-table that the generator will render + # itself must not be stored, or it would freeze later rate_limit / WAF + # window changes (Options are emitted first and win the dedup). + fe_uses_counter = frontend_uses_counter([ + frontend.get(k) for k in ( + "options", "request_headers", "response_headers", "tcp_request_rules", + "acl_rules", "use_backend_rules", "filters", "log_format", + ) + ]) + frontend["options"], dropped_tables = drop_platform_stick_tables( + frontend.get("options"), fe_regenerates, fe_uses_counter) + for table_line in dropped_tables: + parse_result.warnings = [ + w for w in parse_result.warnings + if not (w.startswith(f"Frontend '{frontend['name']}': stick-table imported as-is") + and table_line in w) + ] + stick_policy_warnings.append( + f"ℹ️ Frontend '{frontend['name']}': stick-table '{table_line}' matches the " + f"table HAProxy OpenManager generates for Rate Limit / WAF rate_limit rules " + f"and is generated automatically for this frontend, so it is not stored." + ) + + parse_result.warnings.extend(stick_policy_warnings) + parse_result.warnings = [ w for w in parse_result.warnings if "_acme_challenge_backend" not in w ] @@ -1166,12 +1235,27 @@ async def parse_bulk_config( update_servers = 0 for frontend in frontends_data: - existing = await conn.fetchrow(""" - SELECT * FROM frontends - WHERE name = $1 AND cluster_id = $2 - ORDER BY is_active DESC - """, frontend["name"], request.cluster_id) - + existing = existing_frontend_rows[frontend["name"]] + imported_options = frontend.get("options") + if existing: + frontend["options"] = merge_options(imported_options, existing.get("options")) + + def _final(field): + # Bulk-create only overwrites a free-form field when the import carries it. + if frontend.get(field): + return frontend.get(field) + return existing.get(field) if existing else None + + enhanced_warnings.extend(stick_advisories( + "Frontend", frontend["name"], frontend.get("options"), imported_options, + { + "Request Headers": _final("request_headers"), + "Response Headers": _final("response_headers"), + "TCP Request Rules": _final("tcp_request_rules"), + }, + platform_regenerates[frontend["name"]], + )) + if existing: # Check if any field has actually changed AND track which fields changed has_changes = False @@ -1202,7 +1286,8 @@ async def parse_bulk_config( has_changes = True changes["maxconn"] = {"old": existing["maxconn"], "new": frontend["maxconn"]} parsed_rh = frontend.get("request_headers") - db_rh = _strip_auto_headers(existing["request_headers"]) + db_rh = strip_auto_headers( + existing["request_headers"], platform_regenerates[frontend["name"]]) if parsed_rh and parsed_rh != db_rh: has_changes = True changes["request_headers"] = {"old": db_rh, "new": parsed_rh} @@ -1292,6 +1377,24 @@ async def parse_bulk_config( WHERE name = $1 AND cluster_id = $2 ORDER BY is_active DESC """, backend["name"], request.cluster_id) + + imported_options = backend.get("options") + if existing: + backend["options"] = merge_options(imported_options, existing.get("options")) + + def _final_be(field): + if backend.get(field): + return backend.get(field) + return existing.get(field) if existing else None + + enhanced_warnings.extend(stick_advisories( + "Backend", backend["name"], backend.get("options"), imported_options, + { + "Request Headers": _final_be("request_headers"), + "Response Headers": _final_be("response_headers"), + }, + False, + )) if existing: # Check if any field has actually changed AND track which fields changed diff --git a/backend/services/haproxy_config.py b/backend/services/haproxy_config.py index 0a55982..bc2601f 100644 --- a/backend/services/haproxy_config.py +++ b/backend/services/haproxy_config.py @@ -1379,7 +1379,27 @@ async def generate_haproxy_config_for_cluster(cluster_id: int, conn: Optional[An logger.info(f"CONFIG GENERATION: Backend '{backend['name']}' has NO ACTIVE SERVERS. Writing backend block (will show as DOWN until servers added).") config_lines.append(f"backend {backend['name']}") - + + # Issue #72: a second `stick-table` in one backend is FATAL in HAProxy + # ("stick-table name ... conflicts with table declared"), which makes the + # agent reject the WHOLE cluster config. Can happen when an imported table + # (options) meets one entered by hand in request/response headers. Keep the + # first one, like the frontend R3.3 dedup; configs with a single table are + # rendered byte-identical. + _be_stick_table_emitted = False + + def _append_be_free_form(line_stripped: str) -> None: + nonlocal _be_stick_table_emitted + if line_stripped.startswith("stick-table"): + if _be_stick_table_emitted: + logger.warning( + f"BACKEND STICK-TABLE DEDUP: skipping duplicate declaration in " + f"backend '{backend['name']}': {line_stripped}" + ) + return + _be_stick_table_emitted = True + config_lines.append(f" {line_stripped}") + # --- HAProxy Configuration Best Practice Order --- # 1. Mode and balance method if backend.get('balance_method'): @@ -1396,7 +1416,7 @@ async def generate_haproxy_config_for_cluster(cluster_id: int, conn: Optional[An if line_stripped and line_stripped not in ('[]', '{}', 'null', 'None'): # Lines are complete HAProxy option directives # Examples: "option http-keep-alive", "option forwardfor" - config_lines.append(f" {line_stripped}") + _append_be_free_form(line_stripped) # 3. Health check configuration if backend.get('health_check_uri'): @@ -1452,7 +1472,7 @@ async def generate_haproxy_config_for_cluster(cluster_id: int, conn: Optional[An # Skip empty strings, "[]", or invalid rules if line_stripped and line_stripped not in ('[]', '{}', 'null', 'None'): # Lines are already complete directives (e.g., "http-request set-header X-Backend test") - config_lines.append(f" {line_stripped}") + _append_be_free_form(line_stripped) # 9. HTTP Response Headers if backend.get('response_headers'): @@ -1461,7 +1481,7 @@ async def generate_haproxy_config_for_cluster(cluster_id: int, conn: Optional[An # Skip empty strings, "[]", or invalid rules if line_stripped and line_stripped not in ('[]', '{}', 'null', 'None'): # Lines are already complete directives (e.g., "http-response add-header X-Powered-By Backend") - config_lines.append(f" {line_stripped}") + _append_be_free_form(line_stripped) # Get all servers for this backend (including inactive ones for commenting) # CRITICAL FIX: Handle servers with NULL cluster_id (legacy data) diff --git a/backend/tests/test_issue72_stick_table_import.py b/backend/tests/test_issue72_stick_table_import.py new file mode 100644 index 0000000..fb058b2 --- /dev/null +++ b/backend/tests/test_issue72_stick_table_import.py @@ -0,0 +1,300 @@ +""" +Issue #72 regression tests: `stick-table` re-definition ignored by Config Import. + +Bug: the bulk-config parser had no branch for `stick-table` (frontend and +backend) nor for backend `stick on|match|store-*`, so they were dropped without +a warning. A frontend that kept its `track-sc0` / counter rules then got the +renderer's stick-table auto-inject (`type ip size 100k expire 30s store +http_req_rate(10s)`), i.e. the operator's new table was silently replaced by +the default. Separately, the import preview stripped EVERY `track-sc0 src` / +`sc_http_req_rate(0)` line as "auto-generated", deleting operator trackers. + +DB-free: parser behaviour, the import policy helpers and source guards for the +renderer / endpoint wiring. +""" +import dataclasses +import os +import sys + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from utils.haproxy_config_parser import parse_haproxy_config +from utils.stick_table_import import ( + drop_platform_stick_tables, + frontend_uses_counter, + is_platform_rate_limit_header, + merge_options, + stick_advisories, + strip_auto_headers, + uses_stick_counter, +) + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + +NEW_TABLE = "stick-table type ip size 200k expire 3h store http_req_rate(10s),gpc0_rate(10s),gpt0" +DEFAULT_TABLE = "stick-table type ip size 100k expire 30s store http_req_rate(10s)" + +ISSUE_72_CONFIG = f""" +frontend fe_web + bind *:80 + mode http + option httplog + {NEW_TABLE} + http-request track-sc0 src + http-request deny if {{ sc0_gpc0_rate gt 20 }} + http-response sc-inc-gpc0(0) if {{ status 404 }} + default_backend be_web + +backend be_web + mode http + stick-table type ip size 1m expire 30m + stick on src + stick match src table fe_web + stick store-request src + server s1 10.0.0.1:80 check +""" + + +def _src(rel): + with open(os.path.join(ROOT, rel), encoding="utf-8") as f: + return f.read() + + +# ---------------------------------------------------------------- parser + + +def test_parser_keeps_frontend_stick_table_in_options(): + r = parse_haproxy_config(ISSUE_72_CONFIG) + fe = r.frontends[0] + assert fe.options == f"option httplog\n{NEW_TABLE}" + assert any("stick-table imported as-is" in w and NEW_TABLE in w for w in r.warnings) + # The rest of the frontend is parsed exactly as before. + assert fe.request_headers == "http-request track-sc0 src\nhttp-request deny if { sc0_gpc0_rate gt 20 }" + assert fe.response_headers == "http-response sc-inc-gpc0(0) if { status 404 }" + + +def test_parser_keeps_backend_stick_directives_in_order(): + be = parse_haproxy_config(ISSUE_72_CONFIG).backends[0] + assert be.options == ( + "stick-table type ip size 1m expire 30m\n" + "stick on src\n" + "stick match src table fe_web\n" + "stick store-request src" + ) + assert be.mode == "http" + assert [s.server_name for s in be.servers] == ["s1"] + + +def test_parser_ignores_lookalike_directives(): + cfg = """ +backend b + stickiness on + stick-tablex foo + server s1 10.0.0.1:80 +""" + be = parse_haproxy_config(cfg).backends[0] + assert be.options is None + + +def test_parser_unchanged_without_stick_directives(): + cfg = """ +frontend f + bind *:80 + option httplog + http-request set-header X-A 1 + default_backend b +backend b + option forwardfor + server s1 10.0.0.1:80 +""" + r = parse_haproxy_config(cfg) + fe = dataclasses.asdict(r.frontends[0]) + be = dataclasses.asdict(r.backends[0]) + assert fe["options"] == "option httplog" + assert be["options"] == "option forwardfor" + assert not any("stick" in w for w in r.warnings) + + +# ------------------------------------------------- generator-owned headers + + +def test_platform_rate_limit_header_exact_forms(): + assert is_platform_rate_limit_header("http-request track-sc0 src") + assert is_platform_rate_limit_header("http-request deny if { sc_http_req_rate(0) gt 100 }") + assert is_platform_rate_limit_header( + "http-request capture req.hdr(User-Agent) len 64 if { sc_http_req_rate(0) gt 5 }") + assert not is_platform_rate_limit_header("http-request track-sc0 src table other") + assert not is_platform_rate_limit_header("http-request track-sc1 src") + assert not is_platform_rate_limit_header( + "http-request deny deny_status 429 if { sc_http_req_rate(0) gt 100 }") + + +def test_strip_keeps_operator_rate_limit_lines_when_platform_does_not_regenerate(): + rh = "http-request track-sc0 src\nhttp-request deny if { sc_http_req_rate(0) gt 100 }" + assert strip_auto_headers(rh, platform_regenerates=False) == rh + + +def test_strip_removes_generator_lines_when_platform_regenerates(): + rh = ("http-request track-sc0 src\n" + "http-request deny if { sc_http_req_rate(0) gt 100 }\n" + "http-request set-header X-A 1") + assert strip_auto_headers(rh, platform_regenerates=True) == "http-request set-header X-A 1" + + +def test_strip_keeps_non_exact_forms_even_when_platform_regenerates(): + rh = "http-request track-sc0 src table other\nhttp-request track-sc1 src" + assert strip_auto_headers(rh, platform_regenerates=True) == rh + + +def test_strip_always_removes_acme_and_waf_lines(): + rh = ("http-request allow if is_acme_challenge\n" + "http-request deny if waf_block_1\n" + "http-request set-header X-A 1") + for regen in (True, False): + assert strip_auto_headers(rh, regen) == "http-request set-header X-A 1" + assert strip_auto_headers(None, True) is None + assert strip_auto_headers("http-request track-sc0 src", True) is None + + +# ------------------------------------------- generator-shaped stick-tables + + +def test_drop_platform_table_when_platform_regenerates(): + opts = f"option httplog\nstick-table type ip size 100k expire 60s store http_req_rate(10s)" + kept, dropped = drop_platform_stick_tables(opts, True, False) + assert kept == "option httplog" + assert dropped == ["stick-table type ip size 100k expire 60s store http_req_rate(10s)"] + + +def test_drop_default_table_when_auto_inject_would_render_it(): + kept, dropped = drop_platform_stick_tables(DEFAULT_TABLE, False, True) + assert kept is None and dropped == [DEFAULT_TABLE] + + +def test_keep_generator_shaped_table_the_platform_would_not_render(): + # 60s with only auto-inject in play: auto-inject would render 30s, so keep it. + t60 = "stick-table type ip size 100k expire 60s store http_req_rate(10s)" + assert drop_platform_stick_tables(t60, False, True) == (t60, []) + # Default table with no counter use: nothing would render it (e.g. `table fe` refs). + assert drop_platform_stick_tables(DEFAULT_TABLE, False, False) == (DEFAULT_TABLE, []) + + +def test_never_drop_operator_table(): + for regen in (True, False): + for counter in (True, False): + assert drop_platform_stick_tables(NEW_TABLE, regen, counter) == (NEW_TABLE, []) + + +def test_frontend_uses_counter_matches_renderer_rule(): + assert uses_stick_counter("http-request track-sc0 src") + assert uses_stick_counter("http-request deny if { sc_http_req_rate(0) gt 1 }") + assert not uses_stick_counter("http-request set-header X-A 1") + assert frontend_uses_counter([None, "option httplog", ["acl a sc_http_req_rate(0) gt 1"]]) + # stick-table lines themselves never count (they are the thing being decided on) + assert not frontend_uses_counter([DEFAULT_TABLE]) + + +def test_uses_stick_counter_is_the_renderer_expression(): + s = _src("services/haproxy_config.py") + assert 'if "track-sc" in stripped or ("sc_" in stripped and "_rate(" in stripped):' in s + m = _src("utils/stick_table_import.py") + assert 'return "track-sc" in s or ("sc_" in s and "_rate(" in s)' in m + + +def test_platform_table_regex_covers_every_generator_form(): + s = _src("services/haproxy_config.py") + assert s.count("stick-table type ip size 100k expire 30s store http_req_rate(10s)") == 2 + assert "stick-table type ip size 100k expire {waf_rule.get('rate_limit_window', 60)}s store http_req_rate(10s)" in s + for line in (DEFAULT_TABLE, "stick-table type ip size 100k expire 120s store http_req_rate(10s)"): + assert drop_platform_stick_tables(line, True, False)[1] == [line] + + +# ------------------------------------------------------------ options merge + + +def test_merge_stick_only_import_keeps_existing_option_lines(): + assert merge_options(NEW_TABLE, "option forwardfor") == f"option forwardfor\n{NEW_TABLE}" + + +def test_merge_option_only_import_keeps_existing_stick_lines(): + assert merge_options("option httplog", f"option forwardfor\n{DEFAULT_TABLE}") == \ + f"option httplog\n{DEFAULT_TABLE}" + + +def test_merge_replaces_each_group_when_imported(): + assert merge_options(f"option httplog\n{NEW_TABLE}", f"option forwardfor\n{DEFAULT_TABLE}") == \ + f"option httplog\n{NEW_TABLE}" + + +def test_merge_without_imported_options_keeps_existing_verbatim(): + assert merge_options(None, "option forwardfor") == "option forwardfor" + assert merge_options(None, None) is None + + +def test_merge_same_lines_returns_existing_unchanged(): + existing = f"{NEW_TABLE}\noption httplog" + assert merge_options(f"option httplog\n{NEW_TABLE}", existing) is existing + + +def test_merge_matches_previous_behaviour_when_no_stick_lines(): + # Pre-#72 bulk import replaced options wholesale when the import had option lines. + assert merge_options("option httplog", "option forwardfor") == "option httplog" + + +# ---------------------------------------------------------------- advisories + + +def test_advisory_table_without_http_req_rate_when_rate_limited(): + t = "stick-table type ip size 200k expire 3h store gpc0_rate(10s),gpt0" + w = stick_advisories("Frontend", "f", t, t, {}, platform_regenerates=True) + assert any("does not store http_req_rate" in x for x in w) + assert not stick_advisories("Frontend", "f", t, t, {}, platform_regenerates=False) + assert not stick_advisories("Frontend", "f", NEW_TABLE, NEW_TABLE, {}, platform_regenerates=True) + + +def test_advisory_duplicate_table_in_other_field(): + w = stick_advisories("Backend", "b", NEW_TABLE, NEW_TABLE, + {"Request Headers": "stick-table type ip size 1m"}, False) + assert any("also stored in Request Headers" in x for x in w) + # Not raised when this import did not bring a table. + assert not stick_advisories("Backend", "b", NEW_TABLE, None, + {"Request Headers": "stick-table type ip size 1m"}, False) + + +def test_advisory_stick_rule_without_table(): + w = stick_advisories("Backend", "b", "stick on src", "stick on src", {}, False) + assert any("unable to find stick-table" in x for x in w) + assert not stick_advisories("Backend", "b", "stick on src table fe", "stick on src table fe", {}, False) + assert not stick_advisories("Backend", "b", "stick-table type ip size 1m\nstick on src", + "stick on src", {}, False) + + +def test_advisory_peers(): + t = "stick-table type ip size 1m peers mypeers" + w = stick_advisories("Backend", "b", t, t, {}, False) + assert any("'mypeers'" in x and "BEFORE the 'defaults'" in x for x in w) + + +# ------------------------------------------------------------ wiring guards + + +def test_backend_renderer_dedups_stick_table(): + s = _src("services/haproxy_config.py") + assert "_be_stick_table_emitted" in s + assert "BACKEND STICK-TABLE DEDUP" in s + block = s[s.index("_append_be_free_form(line_stripped: str)"):s.index("# 3. Health check configuration")] + assert "_append_be_free_form(line_stripped)" in block # options loop routed through it + tail = s[s.index("# 8. HTTP Request Headers"):s.index("# Get all servers for this backend")] + assert tail.count("_append_be_free_form(line_stripped)") == 2 # request + response headers + + +def test_parse_bulk_uses_conditional_policy(): + s = _src("routers/config.py") + fn = s[s.index("async def parse_bulk_config("):s.index('@router.post("/bulk-create")')] + assert '"track-sc0 src" in s' not in fn # the unconditional strip is gone + assert "strip_auto_headers(" in fn and "platform_regenerates[" in fn + assert "drop_platform_stick_tables(" in fn + assert fn.count("merge_options(") == 2 # frontends + backends + assert fn.count("stick_advisories(") == 2 + # Bulgu #82 guard window must still see the cluster access check. + assert "validate_user_cluster_access" in fn[:6000] diff --git a/backend/utils/haproxy_config_parser.py b/backend/utils/haproxy_config_parser.py index fbfe904..38502f5 100644 --- a/backend/utils/haproxy_config_parser.py +++ b/backend/utils/haproxy_config_parser.py @@ -11,6 +11,9 @@ from dataclasses import dataclass, asdict logger = logging.getLogger(__name__) +_STICK_TABLE_LINE_RE = re.compile(r'^stick-table\s') +_STICK_RULE_LINE_RE = re.compile(r'^stick\s+(on|match|store-request|store-response)\s') + @dataclass class ParsedServer: @@ -277,6 +280,17 @@ class HAProxyConfigParser: if re.match(r'^log-format(-sd)?\s', line, re.IGNORECASE): log_format_list.append(line.strip()) continue + # Issue #72: `stick-table` matched no branch below and was dropped + # without a warning, so the renderer's auto-inject replaced the + # operator's table with the 100k/30s default. Kept verbatim in + # `options`; the renderer routes it to the `stick` bucket. + if _STICK_TABLE_LINE_RE.match(line): + options_list.append(line.strip()) + self.warnings.append( + f"Frontend '{name}': stick-table imported as-is into Frontend Options: " + f"'{line.strip()}'." + ) + continue # Parse bind directive # IMPORTANT: Handle multiple bind lines correctly @@ -585,6 +599,17 @@ class HAProxyConfigParser: options_list = [] for line in lines: + # Issue #72: `stick-table` and `stick on|match|store-*` were dropped + # without a warning. Kept verbatim in `options`, which the renderer + # writes as-is. + if _STICK_TABLE_LINE_RE.match(line) or _STICK_RULE_LINE_RE.match(line): + options_list.append(line.strip()) + self.warnings.append( + f"Backend '{name}': stick directive imported as-is into Backend Options: " + f"'{line.strip()}'." + ) + continue + # Parse mode mode_match = re.match(r'^mode\s+(\S+)', line, re.IGNORECASE) if mode_match: diff --git a/backend/utils/stick_table_import.py b/backend/utils/stick_table_import.py new file mode 100644 index 0000000..20d581c --- /dev/null +++ b/backend/utils/stick_table_import.py @@ -0,0 +1,219 @@ +""" +Issue #72 — Config Import policy for stick-table / stick rules and the rate-limit +lines the config generator injects. + +Pure functions (no DB) so the import endpoint stays thin and the policy is unit +testable. "Platform regenerates" means the generator will itself emit the +rate-limit table/tracker/deny lines for a frontend: its `rate_limit` field is set +or an active `rate_limit` WAF rule applies to it (see services/haproxy_config.py). +""" + +import re +from typing import Iterable, List, Optional, Tuple + +# The only stick-table forms the generator emits (services/haproxy_config.py: +# frontend rate_limit, stick-table auto-inject, WAF rate_limit rule). +PLATFORM_STICK_TABLE_RE = re.compile( + r'^stick-table\s+type\s+ip\s+size\s+100k\s+expire\s+(\d+)s\s+store\s+http_req_rate\(10s\)$' +) +AUTO_INJECT_EXPIRE_SECONDS = 30 + +# Exact forms of the rate-limit request lines the generator emits. +_PLATFORM_RATE_LIMIT_HEADER_RES = ( + re.compile(r'^http-request\s+track-sc0\s+src$'), + re.compile(r'^http-request\s+deny\s+if\s+\{\s*sc_http_req_rate\(0\)\s+gt\s+\d+\s*\}$'), + re.compile( + r'^http-request\s+capture\s+req\.hdr\(User-Agent\)\s+len\s+64\s+if\s+' + r'\{\s*sc_http_req_rate\(0\)\s+gt\s+\d+\s*\}$' + ), +) + +_STICK_RULE_RE = re.compile(r'^stick\s+(on|match|store-request|store-response)\s') +_TABLE_ARG_RE = re.compile(r'(?:^|\s)table\s+\S+') +_PEERS_ARG_RE = re.compile(r'(?:^|\s)peers\s+(\S+)') + + +def _lines(text: Optional[str]) -> List[str]: + if not text: + return [] + return [l.strip() for l in text.split('\n') if l.strip()] + + +def is_stick_line(line: str) -> bool: + s = line.strip() + return s.startswith('stick-table') or s.startswith('stick ') + + +def is_stick_table_line(line: str) -> bool: + return line.strip().startswith('stick-table') + + +def uses_stick_counter(line: str) -> bool: + """Same test the renderer uses to decide a frontend needs a stick-table + (services/haproxy_config.py `_emit_fe`).""" + s = line.strip() + return "track-sc" in s or ("sc_" in s and "_rate(" in s) + + +def is_platform_rate_limit_header(line: str) -> bool: + s = line.strip() + return any(r.match(s) for r in _PLATFORM_RATE_LIMIT_HEADER_RES) + + +def is_always_auto_header(line: str) -> bool: + """ACME and WAF lines are generator-only: stripped regardless of context.""" + s = line.strip() + if "is_acme_challenge" in s: + return True + if s.startswith("http-request") and " waf_" in s: + return True + return False + + +def strip_auto_headers(headers: Optional[str], platform_regenerates: bool) -> Optional[str]: + """Drop generator-owned lines from a `request_headers` value. + + Rate-limit lines are dropped only when the generator will emit them again for + this frontend AND they match its exact form; otherwise they are the + operator's own (e.g. a tracker feeding gpc0 rules) and are kept. + """ + if not headers: + return None + kept = [] + for line in headers.split("\n"): + if is_always_auto_header(line): + continue + if platform_regenerates and is_platform_rate_limit_header(line): + continue + kept.append(line) + return "\n".join(kept) if kept else None + + +def drop_platform_stick_tables( + options: Optional[str], + platform_regenerates: bool, + frontend_uses_counter: bool, +) -> Tuple[Optional[str], List[str]]: + """Remove imported stick-table lines the generator will reproduce itself. + + Keeping them would freeze them in `options` (emitted first, so they win the + dedup) and hide later rate_limit / WAF window changes. A generator-shaped + line is dropped when rate_limit/WAF will emit a table, or when it is the + 30s auto-inject default and the frontend uses a counter (auto-inject then + renders the identical line). Anything else is the operator's table. + """ + if not options: + return options, [] + kept, dropped = [], [] + for line in options.split("\n"): + m = PLATFORM_STICK_TABLE_RE.match(line.strip()) + if m and ( + platform_regenerates + or (int(m.group(1)) == AUTO_INJECT_EXPIRE_SECONDS and frontend_uses_counter) + ): + dropped.append(line.strip()) + continue + kept.append(line) + return ("\n".join(kept) if kept else None), dropped + + +def merge_options(parsed: Optional[str], existing: Optional[str]) -> Optional[str]: + """Merge an imported `options` value into the stored one per directive type. + + Bulk import only overwrote `options` when the import carried `option` lines; + with stick lines now landing in `options` too, a stick-only import must not + wipe hand-entered `option` lines (and vice versa). Each group is replaced + only when the import carries that group. Returns `existing` unchanged when + the merged lines are the same set, so re-imports don't flag a change. + """ + parsed_lines = _lines(parsed) + existing_lines = _lines(existing) + parsed_other = [l for l in parsed_lines if not is_stick_line(l)] + parsed_stick = [l for l in parsed_lines if is_stick_line(l)] + existing_other = [l for l in existing_lines if not is_stick_line(l)] + existing_stick = [l for l in existing_lines if is_stick_line(l)] + + final = (parsed_other or existing_other) + (parsed_stick or existing_stick) + if not final: + return None + if sorted(final) == sorted(existing_lines): + return existing + return "\n".join(final) + + +def frontend_uses_counter(fields: Iterable) -> bool: + """True if any non-stick line of the given frontend fields uses a counter.""" + for value in fields: + if not value: + continue + items = value if isinstance(value, (list, tuple)) else str(value).split("\n") + for line in items: + if isinstance(line, str) and not is_stick_line(line) and uses_stick_counter(line): + return True + return False + + +def stick_advisories( + kind: str, + name: str, + final_options: Optional[str], + imported_options: Optional[str], + final_other_fields: dict, + platform_regenerates: bool, +) -> List[str]: + """Non-blocking import-preview warnings for stick directives. + + kind: 'Frontend' or 'Backend'. final_other_fields: {label: final value} of + the other free-form fields (what they hold after this import is applied). + """ + warnings: List[str] = [] + final_lines = _lines(final_options) + tables = [l for l in final_lines if is_stick_table_line(l)] + imported_tables = [l for l in _lines(imported_options) if is_stick_table_line(l)] + + if kind == "Frontend" and platform_regenerates: + for t in tables: + if not PLATFORM_STICK_TABLE_RE.match(t) and "http_req_rate" not in t: + warnings.append( + f"⚠️ Frontend '{name}': the imported stick-table does not store " + f"http_req_rate, but this frontend's Rate Limit / rate_limit WAF rule " + f"checks sc_http_req_rate(0) against it. HAProxy accepts this and the " + f"rate limit silently never triggers. Add http_req_rate(10s) to the " + f"table's 'store' list to keep it working." + ) + + if imported_tables: + for label, value in final_other_fields.items(): + if any(is_stick_table_line(l) for l in _lines(value)): + effect = ( + "only the first stick-table is rendered (Options), so the one in " + f"{label} is ignored" + ) + warnings.append( + f"⚠️ {kind} '{name}': a stick-table is also stored in {label}; " + f"{effect}. Remove it from {label}." + ) + + if kind == "Backend": + has_own_table = bool(tables) or any( + is_stick_table_line(l) for v in final_other_fields.values() for l in _lines(v) + ) + for l in final_lines: + if _STICK_RULE_RE.match(l) and not _TABLE_ARG_RE.search(l) and not has_own_table: + warnings.append( + f"⚠️ Backend '{name}': '{l}' needs a stick-table in this backend or a " + f"'table ' argument. HAProxy rejects it at apply " + f"(\"unable to find stick-table\"); the previous config keeps running." + ) + + for t in tables: + m = _PEERS_ARG_RE.search(t) + if m: + warnings.append( + f"ℹ️ {kind} '{name}': stick-table references peers section " + f"'{m.group(1)}'. HAProxy OpenManager does not manage peers sections: define " + f"it in each node's haproxy.cfg BEFORE the 'defaults' section (the agent " + f"keeps everything above the first defaults/listen/frontend/backend and " + f"replaces the rest)." + ) + return warnings diff --git a/backend/version.json b/backend/version.json index 25f6472..0340066 100644 --- a/backend/version.json +++ b/backend/version.json @@ -1,5 +1,5 @@ { - "version": "1.11.2", - "releaseName": "Read-only roles read SSL without pool access; private keys need ssl.download", - "releaseDate": "2026-10-01" + "version": "1.11.3", + "releaseName": "Config Import keeps stick-table and stick rules instead of dropping them", + "releaseDate": "2026-10-16" } diff --git a/frontend/package.json b/frontend/package.json index e3510ac..d397b37 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "haproxy-openmanager-frontend", - "version": "1.11.2", + "version": "1.11.3", "description": "HAProxy Load Balancer Management UI", "license": "AGPL-3.0-or-later", "dependencies": {