mirror of
https://github.com/taylanbakircioglu/haproxy-openmanager.git
synced 2026-09-21 01:53:24 +00:00
02b1cb2bca
Closes #13, Closes #14. This release squashes the v1.4.0 → v1.5.0 development line. v1.4.0 shipped the ACME stability & enterprise audit (Issues #10/#11/#12). v1.5.0 builds on that foundation with two co-equal headline features plus a 22-round audit campaign hardening the prior configuration surface. License remains MIT for v1.5.0 (relicense to AGPL-3.0 lands in v1.5.2). ------------------------------------------------------------------ HEADLINE FEATURE A — ACME Diagnostic Panel (Issue #13) ------------------------------------------------------------------ A live pre-flight + post-failure diagnostic surface for every ACME order, reachable from the ACME Automation page. The panel exists to make ACME failures legible to operators who do NOT have shell access to the API host. Endpoints (`backend/routers/acme_diagnostics.py`): POST /api/letsencrypt/orders/{order_id}/diagnostics Run the full 5-check suite (DNS / port-80 / routing / account / agents) and humanize the order's `error_detail` (>=11 RFC-8555 problem types, backwards compatible with legacy plain-string failures). POST /api/letsencrypt/orders/{order_id}/diagnostics/ {check_id}/rerun Re-run a single check in place — used by the "Re-run" button on every row of the modal's pre-flight table. GET /api/letsencrypt/orders/{order_id}/events Merged event timeline combining the typed `acme_order_events` rows with correlated `user_activity_logs` entries (resource_type = 'letsencrypt_order' AND resource_id = order_id). The diagnostic modal auto-tails this timeline every 5 seconds while open. Service-level checks (`backend/services/acme_diagnostics.py`): * DNS resolution via stdlib socket.gethostbyname_ex through run_in_executor (intentionally avoiding an aiodns runtime dep for v1.5.0). * Port-80 HEAD probe, target locked to the order's domains, success on HTTP 200 OR 404, warns on egress timeout (corp egress policies routinely blackhole outbound 80 — fail-hard would be too noisy). * SSRF guard: probe refuses non-public IPs and surfaces the skip in the diagnostic result; IPv4-mapped IPv6 normalisation closes the `::ffff:169.254.169.254` cloud-metadata vector. * HAProxy routing presence check: matches the order's cluster_ids to a port-80 HTTP frontend. * ACME account validity check against `letsencrypt_accounts`. * Agent presence check (>=1 active agent in target cluster). * Every sub-check wrapped in a wall-clock timeout to bound impact on the API event loop. RBAC: ssl.read for run, ssl.read for events. Per-user 5/min rate limit on both run and rerun, backed by the (user_id, action, created_at DESC) composite index. Frontend (`frontend/src/components/ACMEAutomation.js`): * "Diagnose" button on every order row + the existing "stuck order" warning row. * Modal with two tabs: - Pre-flight Checks (Antd Table with status pills + Re-run buttons + humanized error banner) - Event Log (Antd Timeline with auto-tail polling, scroll- to-bottom, pause-on-hover) * Correlation IDs surfaced in error banners and individual check fail details for backend-log lookup. ------------------------------------------------------------------ HEADLINE FEATURE B — Site Setup Wizard (Issue #14) ------------------------------------------------------------------ A single guided flow that creates a Backend + Servers + HTTP Frontend (and optional HTTPS Frontend) in one atomic transaction. Endpoints (`backend/routers/site_wizard.py`): POST /api/site-wizard/preview — diff-preview the changeset POST /api/site-wizard/create — atomic execute POST /api/site-wizard/reject — clean rollback (including any wizard_staged ACME orders) GET /api/site-wizard/drafts — draft persistence PUT /api/site-wizard/drafts/{id} — save/update DELETE /api/site-wizard/drafts/{id} Feature surface: * One screen captures both backend (mode + servers) AND frontend (http + optional https + SSL mode) inputs. * SSL modes: ACME (new order, HTTP-01 only for v1.5.0), Upload (existing PEM), Existing (link to a stored cert), or None. * ACME-staged path: wizard_staged_until watermark on the `letsencrypt_orders` row defers finalisation until agent confirmation; per-mode reject cleanly cancels and rolls back the staged order. * Live diff preview against the cluster's current generated config (renderer-evolution noise stripped — track-sc<N> dedup, per-server cookie strip, defaults-cookie inheritance, listen-block flattening). * Draft persistence with PEM stripped at save time (private keys never round-trip through the drafts table). * Per-cluster multi-tenancy: drafts and wizard_staged orders are isolated to the creating user's cluster scope. Frontend (`frontend/src/components/SiteWizard.js`): * 4-step Antd Steps flow: Backend → Frontend → SSL → Review. * Render the live diff preview inline before commit. * Antd Form-level validation mirrors backend Pydantic validators (numeric bounds, HAProxy reserved keywords, ALPN consistency, IPv6 scope-id, domain regex, server name dedup). ------------------------------------------------------------------ AUDIT CAMPAIGN — Rounds 1 → 22 (Bulgu #1 → #82) ------------------------------------------------------------------ v1.5.0 includes 22 adversarial review passes. Each round produced its own commit set in the corporate development line; this squash collapses those into the v1.5.0 release artefact. Highlights: Round 1-4 Site Wizard core: dry-run parity, single-line value injection guard, ACL -f pattern-file block, SSL parity, timeout regex, form-state pin. Round 5-7 defaults-cookie inheritance, server-named-cookie guard, fe/be mode mismatch, duplicate server names, health_check_uri + server_address validators. Round 8-10 cookie_name / cookie_options newline-injection guard, dry-run parity (round 9), TCP-mode HTTP-only feature blockers. Round 11 SSL name path traversal + health-check >= 1. Round 12-13 SSL & ACME deep dive (Bulgu #23-#32). Round 14 single-line value injection (Bulgu #33). Round 15-17 ACME multi-tenant UX, numeric bounds, HAProxy reserved keywords, ALPN/TLS consistency, all-backup, multi-domain & multi-user enterprise edges, drain/HSTS/post-completion (Bulgu #34-#53). Round 18-21 concurrency, agent state, TCP-mode HTTP-only, list size caps, IPv6 scope-id, preview account validation, TCP backend + balance uri reject (Bulgu #54-#61). Round 22 FE error visibility + 3x stale-data lockouts, referential integrity + cascade safety, authentication & authorization, multi-cluster isolation, apply_pending_changes concurrency, script injection + bulk import multi-tenancy, prefix-stripped signature comparison (Bulgu #62-#82). ------------------------------------------------------------------ NO CORPORATE-SPECIFIC ARTIFACTS ------------------------------------------------------------------ This squash deliberately sanitises corporate hostnames, container registry references, and TLS secret names into generic placeholders (`your-registry.example.com/your-org`, `haproxy-openmanager*.example.com`, `wildcard-tls`, `taylanbakircioglu/haproxy-openmanager-*`) so the public artefact contains no internal infrastructure detail. Pilot / development history that retained those values stays in the corporate fork and is NOT part of this commit.
251 lines
9.6 KiB
Python
251 lines
9.6 KiB
Python
"""
|
|
v1.5.0 Feature B — _execute_post_completion_actions unit tests.
|
|
|
|
Coverage targets the high-risk gates that survived the multi-round audit:
|
|
* idempotency (M2/M3): an action with executed_at must be skipped silently
|
|
* port-collision pre-check (M21/R35): pre-INSERT collision check rejects
|
|
cleanly without writing
|
|
* non-dict / unknown-type actions are skipped (defensive parsing)
|
|
* renewal of a cert whose order has post_completion_actions DOES NOT
|
|
re-execute them (handled in _complete_certificate's caller path: the
|
|
is_renewal flag gates the call; we cover that contract here)
|
|
"""
|
|
import json
|
|
from contextlib import asynccontextmanager
|
|
from types import SimpleNamespace
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
|
|
# ----------------------------------------------------------------------------
|
|
# Test helpers
|
|
# ----------------------------------------------------------------------------
|
|
|
|
|
|
@asynccontextmanager
|
|
async def _fake_tx():
|
|
"""Stand-in for conn.transaction() async context manager."""
|
|
yield
|
|
|
|
|
|
def _make_conn():
|
|
conn = AsyncMock()
|
|
conn.transaction = MagicMock(side_effect=lambda: _fake_tx())
|
|
return conn
|
|
|
|
|
|
# ----------------------------------------------------------------------------
|
|
# Idempotency
|
|
# ----------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_already_executed_action_is_skipped():
|
|
from routers.letsencrypt import _execute_post_completion_actions
|
|
conn = _make_conn()
|
|
|
|
actions = [{
|
|
"type": "create_frontend",
|
|
"executed_at": "2026-05-01T00:00:00Z", # already done
|
|
"frontend_config": {"cluster_id": 1, "name": "fe-https"},
|
|
}]
|
|
|
|
outcomes = await _execute_post_completion_actions(
|
|
conn, order_id=1, actions=actions, cert_id=42,
|
|
)
|
|
assert len(outcomes) == 1
|
|
assert outcomes[0]["status"] == "skipped"
|
|
assert "already executed" in outcomes[0]["reason"]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_non_dict_action_is_skipped():
|
|
from routers.letsencrypt import _execute_post_completion_actions
|
|
conn = _make_conn()
|
|
|
|
outcomes = await _execute_post_completion_actions(
|
|
conn, order_id=1, actions=["not a dict", 42, None], cert_id=42,
|
|
)
|
|
assert len(outcomes) == 3
|
|
assert all(o["status"] == "skipped" for o in outcomes)
|
|
|
|
|
|
# ----------------------------------------------------------------------------
|
|
# Port-collision pre-check (M21/R35)
|
|
# ----------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_port_collision_skips_create_and_records_event():
|
|
"""If the bind port is taken at the moment of post-completion, we must
|
|
NEVER attempt the INSERT — that would orphan the cert.
|
|
"""
|
|
from routers.letsencrypt import _execute_post_completion_actions
|
|
conn = _make_conn()
|
|
|
|
actions = [{
|
|
"type": "create_frontend",
|
|
"frontend_config": {
|
|
"cluster_id": 1,
|
|
"bind_address": "*",
|
|
"bind_port": 443,
|
|
"name": "fe-https",
|
|
},
|
|
}]
|
|
|
|
record_event_mock = AsyncMock()
|
|
|
|
with patch(
|
|
"services.frontend_service.check_bind_port_collision",
|
|
AsyncMock(return_value=99), # collision: existing frontend id 99
|
|
), patch(
|
|
"services.frontend_service.create_frontend_row",
|
|
AsyncMock(),
|
|
) as create_fe, patch(
|
|
"utils.activity_log.record_event",
|
|
record_event_mock,
|
|
):
|
|
outcomes = await _execute_post_completion_actions(
|
|
conn, order_id=1, actions=actions, cert_id=42,
|
|
)
|
|
|
|
create_fe.assert_not_called()
|
|
assert outcomes[0]["status"] == "error"
|
|
assert outcomes[0]["reason"] == "port_collision"
|
|
|
|
# An event was recorded for the skip
|
|
record_event_mock.assert_awaited()
|
|
awaited_args = record_event_mock.await_args.args
|
|
awaited_kwargs = record_event_mock.await_args.kwargs
|
|
# First positional is order_id, second is event_type
|
|
assert awaited_args[0] == 1
|
|
assert "skipped" in awaited_args[1]
|
|
|
|
|
|
# ----------------------------------------------------------------------------
|
|
# Unknown action type does not crash the loop
|
|
# ----------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_unknown_action_type_does_not_raise():
|
|
from routers.letsencrypt import _execute_post_completion_actions
|
|
conn = _make_conn()
|
|
|
|
# Action with no recognized type — must not crash; outcome flagged.
|
|
actions = [{"type": "redact_secrets"}]
|
|
outcomes = await _execute_post_completion_actions(
|
|
conn, order_id=1, actions=actions, cert_id=42,
|
|
)
|
|
assert len(outcomes) == 1
|
|
# Defensive parsing: at minimum a status is returned per action.
|
|
assert "status" in outcomes[0]
|
|
|
|
|
|
# ----------------------------------------------------------------------------
|
|
# Contract: renewal does NOT execute post-completion actions
|
|
# (the gate lives in _complete_certificate; we assert via grep that
|
|
# `is_renewal` guards the actions call)
|
|
# ----------------------------------------------------------------------------
|
|
|
|
|
|
def test_complete_certificate_renewal_gate_exists_in_source():
|
|
"""The renewal of a cert (where the order is being re-issued via the
|
|
auto-renew daemon) MUST NOT trigger post_completion_actions a second
|
|
time. The gate is `if not is_renewal:` inside _complete_certificate.
|
|
|
|
This is a static assertion against the source — refactor that drops the
|
|
gate would silently re-create the wizard's HTTPS frontend on every
|
|
renewal.
|
|
"""
|
|
from pathlib import Path
|
|
src = Path(__file__).resolve().parent.parent / "routers" / "letsencrypt.py"
|
|
text = src.read_text()
|
|
assert "is_renewal" in text
|
|
# Specifically inside _complete_certificate, post_completion_actions
|
|
# is invoked under the `if not is_renewal:` guard.
|
|
idx = text.find("_execute_post_completion_actions")
|
|
assert idx != -1, "post-completion call must exist in _complete_certificate"
|
|
# Look back a few hundred chars for the renewal guard.
|
|
window_start = max(0, idx - 800)
|
|
surrounding = text[window_start:idx]
|
|
assert ("is_renewal" in surrounding or "not is_renewal" in surrounding), (
|
|
"_execute_post_completion_actions must be gated by 'is_renewal' to "
|
|
"prevent re-running wizard actions on renewal."
|
|
)
|
|
|
|
|
|
def test_wizard_order_bypasses_existing_cert_match():
|
|
"""Bulgu #4: a wizard-staged order with non-empty post_completion_actions
|
|
must NOT be merged into a manually-issued cert that happens to share
|
|
the same primary_domain. The is_wizard_order short-circuit forces
|
|
existing_cert=None so a fresh cert row is INSERTed and the deferred
|
|
HTTPS frontend creation actually fires.
|
|
|
|
Static source assertion — a refactor that drops `is_wizard_order` would
|
|
silently regress wizard host creation when an old manual cert exists.
|
|
"""
|
|
from pathlib import Path
|
|
src = Path(__file__).resolve().parent.parent / "routers" / "letsencrypt.py"
|
|
text = src.read_text()
|
|
assert "is_wizard_order" in text, (
|
|
"_complete_certificate must short-circuit existing_cert lookup "
|
|
"when post_completion_actions is non-empty (Bulgu #4)"
|
|
)
|
|
|
|
|
|
def test_wizard_uses_consolidated_version_for_acme_gating():
|
|
"""Bulgu #30: the staged ACME order's pending_apply_version_name must be
|
|
the CONSOLIDATED version name returned by apply_cluster_pending
|
|
(`apply-consolidated-{ts}`), NOT the original PENDING version name
|
|
(`bulk-proxied-host-create-{ts}`). Agents only ever report the
|
|
consolidated name back via /config-applied, so gating on the bulk
|
|
name would block wizard order promotion forever.
|
|
"""
|
|
from pathlib import Path
|
|
src = Path(__file__).resolve().parent.parent / "routers" / "site_wizard.py"
|
|
text = src.read_text()
|
|
assert "gating_version_name" in text, (
|
|
"create_proxied_host must derive a gating version name from "
|
|
"apply_result.latest_version (Bulgu #30)"
|
|
)
|
|
# The staged order must be created with gating_version_name, not
|
|
# the literal `version_name` (which is the PENDING bulk name).
|
|
idx = text.find("create_order_staged(")
|
|
assert idx != -1
|
|
# Look forward 1000 chars for the pending_apply_version_name= kwarg.
|
|
snippet = text[idx:idx + 1500]
|
|
assert "pending_apply_version_name=gating_version_name" in snippet, (
|
|
"create_order_staged must be called with pending_apply_version_name="
|
|
"gating_version_name (Bulgu #30)"
|
|
)
|
|
|
|
|
|
def test_main_processes_wizard_staged_orders_unconditionally():
|
|
"""Bulgu #2: _process_wizard_staged_orders must run every cycle of
|
|
complete_pending_acme_orders, even when no claimed_ids exist. Otherwise
|
|
a freshly created wizard order on an idle system would never leave
|
|
wizard_staged status.
|
|
|
|
Static source assertion against main.py: the call to
|
|
_process_wizard_staged_orders should NOT sit after the
|
|
`if not claimed_ids: continue` early-out.
|
|
"""
|
|
from pathlib import Path
|
|
src = Path(__file__).resolve().parent.parent / "main.py"
|
|
text = src.read_text()
|
|
process_call_idx = text.find("_process_wizard_staged_orders(acme_svc)")
|
|
assert process_call_idx != -1, (
|
|
"_process_wizard_staged_orders must be invoked from "
|
|
"complete_pending_acme_orders"
|
|
)
|
|
early_continue_idx = text.find("if not claimed_ids:")
|
|
assert early_continue_idx != -1
|
|
# The wizard processing call must come BEFORE the early-out so the
|
|
# idle-system path still drives wizard_staged orders forward.
|
|
assert process_call_idx < early_continue_idx, (
|
|
"_process_wizard_staged_orders must run BEFORE 'if not claimed_ids: continue' "
|
|
"(Bulgu #2: otherwise wizard orders are starved on idle systems)"
|
|
)
|