mirror of
https://github.com/taylanbakircioglu/haproxy-openmanager.git
synced 2026-09-11 21:38:55 +00:00
main
292 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 taylanbakircioglu
|
0ee227363e |
fix(agent): adoption cannot hide a node; Config Import on fresh installs (v1.11.1)
Six fixes to the agent's discovery path and one long-standing parity gap, found
by auditing it in loops against a real fleet.
DISCOVERY REPORTS THAT NEVER REACHED THE SERVER
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, and curl without -f
exits 0 on 5xx too, so a report the server REJECTED was recorded as delivered.
Since a hand-maintained config does not change on its own, that node dropped out
of "Unmanaged keepalived detected" permanently; the only cure was deleting a
cache file on the node by hand.
- the report is cached only on a 2xx;
- GET /agents/{name}/keepalived-config now reports whether the server actually
holds a discovery for that agent, and the cache may only suppress while it
says yes - which is what lets nodes stuck from earlier releases recover on
their own, with nobody touching them;
- the flag is parsed with has() + tostring, not `// empty`: jq's alternative
operator returns the alternative for **false** as well as null, so the naive
form could not tell "no record" from "older backend" and the recovery would
have been completely inert;
- a 400/413/422 records the refusal so identical bytes are not re-posted
forever - 4xx and 5xx agent calls 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 here they mean a token
rotation or an agent row briefly absent, not a bad payload;
- the CLEAR path had the same exit-code defect, where it left a stale row
offering a managed node for adoption with nothing to ever retry it.
CONFIG IMPORT WAS A NO-OP ON FRESHLY INSTALLED AGENTS
check_config_requests uploads a node's live haproxy.cfg on request. It was
defined in the installer body and in the self-upgrade daemon, but not in the
heredoc a fresh install writes, and its call site is guarded by `type` - so on
such a node the operator asked for a config and nothing arrived, with no error
anywhere. Any agent that had self-upgraded at least once already had it, which
is why it went unnoticed. The self-upgrade definition is copied verbatim
(verified line-for-line). A freshly installed agent now polls that endpoint once
per cycle exactly as every upgraded agent already does; no node running today
changes behaviour.
DETERMINISTIC CONFIG PATH
A pool may hold several clusters and the join that resolves keepalived_config_path
was unordered, so the path handed to an agent could differ between polls whenever
two clusters disagreed - the agent would inspect a file that is not there and the
node would never appear, intermittently. A customised path now wins over the
shipped default, then the lowest cluster id. Verified against a real PostgreSQL
over seven arrangements: with one cluster per pool, or when every cluster carries
the default, the value is byte-identical to before.
Verified end to end on a production fleet and, for each decision, against the
real _kp_discover block rather than a paraphrase.
Backend suite: 1674 passed, 152 skipped. bash -n passes on the whole file and on
the fresh-install body in isolation. The keepalived path is logic-identical
across both daemon copies, now pinned by a test.
v1.11.1
|
||
|
taylanbakircioglu
|
9d7a142cfa |
test(requestlog): make the byte-budget drain test independent of runner speed
My own test, and it broke the release build. CI reported `assert 239800 == 0` on the very commit that was supposed to ship v1.11.0, so no image was pushed and the tag and release were never cut. The test drained a 50-row queue with `batch_size=100` and `flush_ms=10`, then asserted the byte counter was back to zero. `_collect()` stops at whichever comes first, `batch_size` rows or the flush deadline - and with a batch size larger than the row count, the deadline is the only thing that can end it. It was measuring the scheduler, not the sink. The arithmetic is exact: a row here weighs 1400 + 4096 + 4096 = 9592 bytes, and 239 800 is 25 of them. `_collect()` returned half the queue because 25 iterations of `asyncio.wait_for` were enough to exhaust 10 ms on that runner. The workflow builds `linux/amd64,linux/arm64`, so one of the two runs under qemu emulation; a local `docker build` compiles the native platform only and never sees that path. I could not reproduce the failure even building both platforms here - this machine fits 49 iterations inside 10 ms - which is the point: a test whose result depends on how fast the host is will pass everywhere it is convenient and fail where it matters. Fixed structurally rather than by widening the window: `batch_size` now EQUALS the row count, so the collect loop exits on the count and never consults the deadline at all. The flush window is generous as a backstop, the drain runs in a loop instead of a single call, and the row count is asserted on the way in and on the way out so a future change cannot make it vacuous. Verified on both platforms the workflow builds: 1667 passed / 152 skipped on linux/arm64 and on linux/amd64 under emulation. No production code changes.v1.11.0 |
||
|
taylanbakircioglu
|
5f995d9d58 |
refactor(ui): move Request Log next to Settings in the sidebar
The page and its policy are two halves of one feature: what gets captured and how long it is kept lives in Settings -> Request Log, and the log itself is the page. Sitting between Config Versions and Clusters put the two at opposite ends of the menu. Now directly above Settings. Menu placement only. The route, the page, the permission gate and every backend behaviour are unchanged. |
||
|
taylanbakircioglu
|
5f5c7f1c75 |
docs(v1.11.0): document what actually ships, with the measurements behind it
The release notes inherited from the feature branch described the version it was written against, not the one going out. - `SCHEMA_VERSION` is 11 -> 12, not 10 -> 11, and the upgrade notes now say why: 11 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. Includes the no-op recovery path for anyone running a pre-release build that recorded 11. - Successful agent polls are not logged by default, with the measured table behind it: 2 424 bytes/row on PostgreSQL 15 against the real schema and all nine indexes, ~9 792 logged calls/day/agent, and what that means at 20, 200 and 500 nodes both ways. The point is not the disk, it is that the row cap holds by DELETING, so without this the configured 7-day/30-day retention quietly becomes a few hours for everything in the table. - Runtime cost stated as measured numbers rather than adjectives: 27.7 us per request, 1.4 us on an excluded path, 18.8 us per row on the writer, 0.096 % of one core at 500 nodes. - REQUEST_LOG_QUEUE_MAX_BYTES documented in .env.template and CONFIG.md, with the reason it exists: the row count alone does not bound memory when max_body_bytes is operator-editable to 256 KB. - The old "raise REQUEST_LOG_QUEUE_MAX if you see drops" advice is corrected - following it could OOM the worker. Lower max_body_bytes or sample_rate first; if you do raise the queue, raise its byte ceiling with it. - Two behaviours that used to be silent are now written down: sink counters are per worker, and clearing the exclude-path list falls back to the shipped defaults rather than logging everything. - The `operator` role's visibility of agent rows is documented, including what it deliberately does NOT extend to (anonymous traffic and the usernames in failed logins). The v1.10.4 through v1.10.14 notes are unchanged and still above this in both files. |
||
|
taylanbakircioglu
|
fbe223250b |
perf(requestlog): gate the embedded-secret scan behind a substring pre-check
The auth_pass / stats-auth / userlist / URI patterns added by the redaction
fixes on this branch run on the writer task, on every string value of every
captured body, so their cost is paid per row forever. Measured, they were:
writer-side redaction, before the redaction fixes 12.2 us/row
writer-side redaction, with them 359.1 us/row
A 29x regression, and none of it was spent matching anything - almost every
body contains none of these keywords. Profiling the four patterns on an 8 KB
config body, 300 iterations:
one combined alternation, IGNORECASE, \b-anchored 308.2 us
the same four run separately (sum) 219.7 us
text.lower() once + four substring pre-checks 5.3 us
`\b` and IGNORECASE each defeat the regex engine's literal-prefix scan, so
every alphanumeric position in 8 KB became a candidate start and the engine
walked the whole body four times to find nothing. Isolated, `auth_pass` costs
27.5 us with IGNORECASE and 2.4 us without.
Split the alternation and gate each pattern behind a substring test on one
lowercased copy. `str.lower()` and `in` are C-level scans; a pattern now runs
only when its keyword is actually present, and then on text that genuinely
contains it. Cost becomes O(total string bytes) instead of O(bytes x patterns).
The URI pattern also drops IGNORECASE and `\b` - its character class already
covers both cases, and `://` gives the engine a literal to scan for.
writer-side redaction, after 18.8 us/row
= 3.4s of CPU/day
at 180 000 rows/day
6.6 us/row over the pre-fix baseline, for four secret classes that were
previously written to the table in cleartext.
The pre-checks are on the lowercased copy, so the patterns stay IGNORECASE: the
marker may well have been `AUTH_PASS` in the original. Request-path cost is
unchanged at 27.7 us p50 - none of this ever ran there.
All 123 redaction and payload tests still pass, so the behaviour is identical;
only the path to it is cheaper.
|
||
|
taylanbakircioglu
|
c5fbbd753f |
test(requestlog): pin the real payloads and the fleet-scale behaviour
The branch shipped 245 tests and 72 of them covered redaction, all passing,
while six real endpoints of this application still wrote secrets to
`request_logs`. That is not a gap in effort, it is a gap in kind: those tests
pin the RULES - which key names match, which value shapes fire - and a rule test
proves the rule, not the coverage. Nothing was measuring what this system
actually sends.
51 tests in two files, every case built from a real handler's request or
response shape with the field names taken from the source and cited in the
docstring.
test_request_log_real_payloads.py drives payloads through `decode_body()`, the
same entry point the writer uses, rather than calling `redact()` on a dict. That
is load-bearing: a config upload is routinely larger than the capture cap, so it
never reaches redaction as a dict at all - it arrives as one truncated `_raw`
string where the line breaks are still the escape `\n`. A test that starts from
a dict reports a pass on a payload that leaks, and on one that gets masked into
uselessness. Both properties are asserted on both paths: the secret is gone AND
the rest of the config is still readable.
test_request_log_fleet_scale.py pins the four behavioural fixes, each of which
only appears at scale or at the edge of a setting's documented range:
* successful agent polls are dropped and failures never are, including a
transport error with no HTTP response at all;
* agent traffic is identified from headers, and `offer()` is asserted to
contain no `await` and no connection call, because it runs on the request
coroutine;
* `requestlog.read` scoping admits agent rows but NOT `user_id IS NULL`, so
anonymous traffic and the usernames in failed logins stay admin-only;
* background passes get one id each, and unwrapped background code does not
collapse onto one either;
* queue memory stays inside its budget with `max_body_bytes` at its 256 KB
ceiling, and the budget is released as rows drain - a budget that only
counts up is a leak, not a limit.
Also closes a hole in the branch's own auth tests: they asserted that every
endpoint calls `_require`, but not that it is called BEFORE the try block. The
repo's GHSA-3p5c pattern exists because a permission check inside `try` is
swallowed by the handler's `except Exception -> 500`, which turns a 403 into a
server error and hides that the check ran. Now asserted per endpoint.
Both halves of each trade are pinned: alongside every "this must be redacted"
there is an "and this must not be", so a later tightening cannot quietly blank
the fields the feature exists to show.
|
||
|
taylanbakircioglu
|
bd4a50943f |
fix(requestlog): bound queue memory, and stop the UI reporting things it cannot know
Three hardening fixes with the same shape: a number that was true under the
defaults and untrue at the edges.
1. QUEUE MEMORY WAS AN OPERATOR SETTING, NOT A LIMIT.
The queue was bounded by ROW COUNT only, and how much a row weighs is
`requestlog.max_body_bytes` - editable from Settings, documented ceiling 256 KB,
and a row can hold that twice (request + response). Measured on the real
dataclass with distinct buffers per row:
defaults, 2 000 rows x 8 KB 33.9 MiB 3.3% of the 1 GiB pod limit
max_body_bytes at its 256 KB ceiling 1003 MiB at the pod limit
REQUEST_LOG_QUEUE_MAX at its ceiling 1695 MiB over the pod limit
Both are reachable from in-range, documented values, and the drop warning
advised "raise REQUEST_LOG_QUEUE_MAX" - so following the tool's own advice on a
busy install could OOM the worker. REQUEST_LOG_QUEUE_MAX_BYTES (default 64 MiB)
now caps the queue in bytes as well as in rows, whichever binds first, released
as rows drain. Verified: with max_body_bytes at 256 KB the queue holds 7.5 MiB
against an 8 MiB budget where it would otherwise have held 1003 MiB, and it
accepts rows again as soon as the writer drains it. The warning text now names
the setting that actually helps.
2. SINK COUNTERS ARE PER WORKER AND DID NOT SAY SO.
The sink is a module global, so with UVICORN_WORKERS > 1 each process has its
own queue and its own counters, and `GET /api/request-logs/stats` reports
whichever worker happened to serve the request. The feature is sold on "a
saturated logger drops rows visibly"; at 4 workers the visible number was a
quarter of the truth. Labelled `"scope": "this worker only"` rather than
aggregated - there is no cross-process channel here, and a number that looks
fleet-wide but is not is worse than one that admits its scope.
3. AN EMPTY EXCLUDE LIST IS NOT APPLIED AS "LOG EVERYTHING".
normalize_exclude_paths() falls back to the shipped defaults when the list comes
out empty, which is the right call - it keeps the log viewer and the raw-body
heartbeat endpoint excluded - but the UI kept displaying the empty list the
operator typed, so the form showed a policy that was not in effect. The save
handler now re-applies whatever the server actually stored (which also surfaces
server-side clamping of every numeric field) and says plainly that the defaults
were restored.
|
||
|
taylanbakircioglu
|
bec0613ae5 |
fix(requestlog): give each background pass its own correlation id
Outbound rows from background work fell back to `bg:<asyncio task name>`.
Nothing in main.py passes `name=` to `create_task`, so every loop keeps one
auto-assigned name - `Task-5` - for its entire life, and every call it ever
makes is written with that same `request_id`. Measured: fifteen ACME calls
across five renewal ticks came out as one id.
That is not a cosmetic grouping problem. `GET /api/request-logs/{id}` returns
every other row sharing the id as `related`, up to 100, and the UI presents
that list as "the calls this request triggered" - it is the feature's headline.
An operator opening a failed renewal was therefore shown up to a hundred
unrelated calls, possibly spanning days, labelled as the trace of the one they
were reading. In a forensics tool a confidently wrong trace is worse than no
trace. Task numbers are reused across restarts too, so `bg:Task-5` could mean a
different loop after a redeploy.
begin_background_trace(label) opens `bg:<label>:<uuid12>` for one iteration and
is called at the top of the three loops that make outbound calls:
complete_pending_acme_orders, check_letsencrypt_renewals, monitor_agent_status.
The loop task is dedicated, so the next iteration overwrites it and there is
nothing to reset.
The fallback for background code that has not been wrapped now mints a unique
id per call instead of reusing the task name. That errs toward too little
grouping rather than too much: a row that stands alone is honest, a row falsely
grouped with a hundred others is not.
Verified: five ticks of three calls produce five distinct ids with the three
calls of each tick sharing one, and four calls from an unwrapped task produce
four distinct ids.
|
||
|
taylanbakircioglu
|
4e2d936c27 |
fix(requestlog): stop the table size from scaling with fleet size
The row rate of `request_logs` was a function of how many nodes are installed,
not of what anyone did. Counted from the agent loop in linux_install.sh, each
agent's 30s cycle issues three logged calls - config, pending-requests,
upgrade-status (the heartbeat is already on the default exclude list) - plus
keepalived-config and keepalived-status every fifth cycle. That is ~9 800
rows/day per agent, essentially all of them 200s meaning "nothing changed".
Measured on PostgreSQL 15 against the real DDL and all nine indexes, at 2 424
bytes/row:
20 agents ~196k rows/day 453 MB/day row cap reached in 2.5 days
200 agents ~2.0M rows/day 4.4 GB/day row cap reached in 6 hours
500 agents ~4.9M rows/day 11 GB/day row cap reached in 2 hours
The cap holds, so nothing runs away - but it holds by deleting, and what it
deletes is everything else. The shipped policy says 7 days of successes and 30
days of failures; on a 200-node fleet it delivers about six HOURS of both. The
forensic record the feature exists for is evicted by polling noise, and the
larger the installation the less history it keeps.
`requestlog.capture_agent_success`, default FALSE: a SUCCESSFUL inbound call
from an agent is not recorded. Failures always are, whatever the flag says -
they are what an operator needs and they are rare, so they cost nothing. With
this the table's size follows operator activity, and adding nodes does not
shorten anyone's retention.
Agent traffic is identified by header only, no database round-trip on the hot
path: the installed agent sends `X-API-Key` and never `Authorization`, the UI
sends a JWT and never an agent key. `generate-install-script`, the one endpoint
that accepts either, classifies correctly under the same rule - an operator
generating a script sends Authorization, a self-upgrading agent sends only the
key. The result is stored in the existing `target` column, which already means
"who was on the other end" for outbound rows and now means the same for inbound
ones, so no schema change and the existing target index applies.
Second half, and the reason this is one commit: `operator` holds
`requestlog.read` because, per the migration that grants it, "operators debug
failing applies and ACME orders". They could not. An apply fails on the NODE,
and the node reports that over its own API key, so the row carrying the
diagnosis has `user_id IS NULL` - and own-rows-only scoping hid it from exactly
the role the grant was written for. Scoping now admits agent rows alongside the
caller's own. Deliberately keyed on `target = 'agent'` rather than `user_id IS
NULL`: anonymous traffic is not agent traffic, so failed logins and their
usernames, and unauthenticated probes, stay admin-only.
Verified end to end through the real middleware: a successful agent poll is
dropped, a 422 from config-validation-failed is kept, operator and anonymous
calls are unaffected, and flipping the setting on restores the old behaviour.
|
||
|
taylanbakircioglu
|
82e6fe3f9c |
fix(requestlog): mask HAProxy credentials in uploaded config bodies
This application never RENDERS a credential into a haproxy.cfg - grepping the
generator and every sample config for `stats auth`, `userlist` and
`insecure-password` returns nothing - so none of our own output is at risk. The
exposure comes from the other direction: the agent uploads the node's REAL
on-disk file.
linux_install.sh: config_content=$(cat "$config_path")
-> POST /api/configuration/agents/{n}/config-response
and `POST /api/config/validate` plus the bulk import take whatever the operator
pastes. A production haproxy.cfg routinely carries `stats auth admin:<password>`
and a `userlist` block, so what is captured is credential-bearing even though
what we generate is not.
Two patterns, folded into the existing single-pass alternation:
stats auth admin:S3cr3t -> stats auth admin:********
user ops password $6$... -> user ops password ********
user dev insecure-password Hunter2 -> user dev insecure-password ********
The username on `stats auth` is deliberately kept: an operator debugging a 401
still needs to know WHICH account it was about.
The load-bearing detail is where a value ENDS. A config upload is routinely
larger than the 8 KB capture cap, so for exactly these payloads the common case
is not the parsed body - it is the truncated `{"_raw": ...}` fallback, where the
line breaks are still the two-character escape `\n` rather than real newlines.
A "rest of the line" match that does not know that runs past every apparent
line break: measured on a 17 KB upload, `auth_pass ...` masked the entire
remainder of the captured string. No leak, but the row is then worthless. Every
value pattern here stops at a real newline OR at a literal backslash-n, so the
same three credentials are masked and the surrounding config stays readable in
both forms. Both paths are verified.
Anchoring is to HAProxy keyword syntax, not to the bare word "password", so
ordinary prose survives: "invalid password format" and "the password must be 8
chars" are untouched. One accepted false positive: "user admin password reset
requested" masks the word "reset", because it is indistinguishable from a
userlist line without parsing the file. That is the same trade the existing
REDACT_CONTAINS entry for `token` already makes - a blanked word costs a little
readability, an unmasked credential costs a credential.
|
||
|
taylanbakircioglu
|
190d45fe09 |
fix(requestlog): scrub credentials carried inside URI values
`POST /api/mfa/enroll` returns the new TOTP secret twice: once as `secret`,
which redaction already caught, and once inside `otpauth_uri` as a query
parameter, which it did not. Blanking one field while the same value sits three
keys away in cleartext is not redaction.
before: {"secret": "***REDACTED***",
"otpauth_uri": "otpauth://totp/OpenManager:admin?secret=JBSWY3DP..."}
after: {"secret": "***REDACTED***",
"otpauth_uri": "otpauth://totp/OpenManager:admin?secret=***REDACTED***"}
routers/mfa.py states the rule this restores: its own activity-log call records
`{"secret_len": len(secret_plain)}` with the comment "NEVER log the secret
itself".
The fix is not otpauth-specific. Any URI found in any captured string value now
goes through scrub_url, which was already written for exactly this and was only
ever pointed at the request line. That also covers:
* userinfo credentials - `https://user:pass@host/path` keeps only the host;
* any query-string credential in a body value or an error message, using the
same is_secret_key rules as the request line, so `?token=`, `?api_key=`,
`?password=` are all handled without naming them again here;
* fragments, which are dropped - they never reach a server and can carry
tokens.
Folded into the existing alternation rather than added as a second pass, so a
captured body is still scanned once. Two guards keep it from doing harm: a URI
with neither `?` nor `@` is returned untouched rather than rebuilt, and if
scrub_url reports a parse failure the original text is kept - inside a larger
string its placeholder would corrupt the surrounding sentence.
Verified: the TOTP secret and a userinfo password are removed, an in-text URL
inside an error message is scrubbed in place, and two innocent ACME URLs
(directory_url, account_url) come through byte-identical.
Also withdrawn here: the review flagged `POST /api/agents/generate-install-script`
as leaking the agent API key through its `script` field. That was wrong. The
finding was built from the endpoint's docstring EXAMPLE (routers/agent.py:497),
which shows an `api_key` field and an `API_KEY="agt_..."` line; the handler's
actual return is {script, platform, cluster_id, filename} and the template
substitution map has no token placeholder. The agent token reaches a node by a
different path entirely, and is issued as `api_key` (routers/security.py:144),
which REDACT_CONTAINS already covers. No change was needed and none is made.
|
||
|
taylanbakircioglu
|
c50408026b |
fix(requestlog): keep the VRRP password out of the request log
Measured against the redaction module from the previous commit, using the real
field names this codebase uses. Three paths wrote the keepalived `auth_pass`
secret to `request_logs` in cleartext:
POST/PUT /api/vip request body, `auth_pass` field
GET /api/agents/{n}/keepalived-config response, keepalived.config_content
POST /api/agents/{n}/keepalived-discovery request body, config_content
The middle one is the worst: the agent polls it on the SSL cadence, so the
secret was re-written to the audit table roughly 576 times a day per member
node.
Two independent holes, closed independently:
1. KEY NAME. `auth_pass` normalizes to "authpass". "password" is not a
substring of it, and the bare "auth" entry in REDACT_EXACT is an exact
match, not a prefix - so nothing in either set matched and the field was
kept verbatim. "authpass" is now in REDACT_EXACT.
2. EMBEDDED IN A CONFIG BLOB. The two agent endpoints do not carry the secret
as its own field at all; they carry a whole keepalived.conf as one string
under `config_content`, with `auth_pass <secret>` on a line inside it. No
key-name rule can see that, and the value-shape guards do not either: it is
neither a PEM block nor a JWT. Added scrub_embedded_secrets(), one compiled
alternation applied in a single pass per string value, masking the whole
remainder of the line so a secret containing whitespace cannot partially
leak - the same pattern and the same reasoning as routers/vip.py's
_redact_secret and routers/agent.py's _AUTH_PASS_MASK_RE.
It runs BEFORE the _MAX_STRING truncation, not after: `auth_pass` sits in the
first few hundred bytes of a rendered keepalived.conf, so letting the cut
"handle" it would be relying on where the secret happens to fall in the file.
This restores an invariant the codebase already states and already enforces
elsewhere. routers/vip.py: "the secret never leaves the server in cleartext ...
only the at-rest Fernet token and the agent-delivery endpoint ever see the real
value". The discovery handler in routers/agent.py goes to three separate
lengths to uphold it - it pops auth_pass out of the parsed analysis, replaces it
with a has_auth_pass boolean, Fernet-encrypts the secret into its own column,
and stores only a masked copy as vip_discoveries.raw_config_masked. Capturing
the request that produced all that, unmasked, put the plaintext right back next
to it.
Verified: the three cases above now redact, and the ten cases that already
worked (login password, JWTs, PEM private keys under any key name, DNS provider
credentials, ACME JWS) are unchanged.
|
||
|
taylanbakircioglu
|
4c84596215 |
feat(logging): unified request/response log with configurable retention
Applies PR #59 by Mustafa Ulukaya (github.com/taylanbakircioglu/haproxy-openmanager/pull/59,
head
|
||
|
taylanbakircioglu
|
1d4e4286af |
fix(agent): keep acknowledging once converged, so a lost report self-heals (v1.10.14)
The deploy report is the server's only evidence that a member node applied its keepalived.conf, and it was sent on the write path alone. Once the rendered config was on disk the agent took the idempotency early return every cycle and never reported again, so a single lost report - a backend restart, a 5xx, a network blip - left the VIP reading SYNCING with an empty "Last ack" forever while the node was demonstrably running the right config. Nothing would ever reconcile the two; the only escape was to change the rendered config so the agent wrote it again, which means touching a live VIP to fix a display problem. The agent now re-asserts its state on that path too: one request per node per poll cycle (~2.5 min), nothing written, keepalived not reloaded. This gap dates from the original HA/VIP work rather than this release series; it only became visible when acknowledgements were dropped for an unrelated reason. A test pins that both daemon copies report BEFORE the early return, since placing it after would silently restore the old behaviour. Verified end to end on a real HA pair: discovery, instance-based adoption of both nodes, PENDING, Apply, agent pull, the validation gate, the hash-pinned takeover, the acknowledgement, and retirement of the one-shot authorisation.v1.10.14 |
||
|
taylanbakircioglu
|
0eb587dfa8 |
fix: keepalived validation gate and deploy acknowledgements (v1.10.13)
Two agent-side fixes found while taking the VIP adoption flow through a real
HA pair, released together.
1. A VALID CONFIG WAS REJECTED BY ITS OWN WARNING (v1.10.12)
Before writing a rendered keepalived.conf the agent validates it with
keepalived -t and, on failure, keeps the running config and does not restart
keepalived. That fail-safe is right, but it treated ANY non-zero exit as
invalid - and keepalived's config-test exit code does not separate fatal from
benign. Measured on 2.2.8:
clean config ..................... 0
auth_pass longer than 8 chars .... 5 "Truncating auth_pass to 8 characters"
missing '}' ...................... 5 "There are 1 missing '}'s"
unknown keyword .................. 5 "Unknown keyword '...'"
script without script_security ... 6 "SECURITY VIOLATION ..."
Exit 5 covers both a harmless truncation and a broken file, so a VRRP password
over eight characters was enough to block every apply - including on a node
whose own running config emits the same warning and had been serving the VIP
for weeks. Accepting exit 5 would have accepted broken configs, so the gate now
judges the OUTPUT: known-benign messages are dropped and anything remaining
still fails. It fails CLOSED - an unrecognised message, or a non-zero exit with
no readable output at all, is fatal - and the filter is an allowlist, never a
denylist. The agent also reports what keepalived said, in its log and in the
status the HA/VIP page shows; discarding it left a correct refusal that nobody
could act on.
2. EVERY DEPLOY ACKNOWLEDGEMENT WAS DROPPED
The takeover-retirement clause on POST /agents/{name}/keepalived-status reused
one placeholder for both the assignment `last_deploy_hash=$n` and the
comparison inside its CASE. PostgreSQL types a placeholder per USE, so it came
out as text in one and character varying in the other and asyncpg rejected the
statement with AmbiguousParameterError. The whole UPDATE never ran, so no
member recorded an acknowledgement: VIPs sat at SYNCING with an empty Last ack
while the nodes were verifiably running the config, and teardown acks were lost
the same way. The hash now has its own placeholder, compared only against the
column.
Verified against real keepalived and a real PostgreSQL rather than by
inspection, including on busybox and bash 3.2, and a test asserts every $n in
those statements is bound exactly once.
|
||
|
taylanbakircioglu
|
9e64002f1c |
fix(vip): make the Adoptable tag name the real blocker (v1.10.11)
The tag and the disabled Adopt button were computed by two separate ladders and could disagree. Seen on a live pair: one node's keepalived.conf had an unbalanced brace, so it was excluded from the instance; its partner was then tagged "MASTER missing" - technically true, because the unreadable node's state MASTER had not been counted - while the actual reason (the peer cannot be taken over, so adopting would strand it) sat only in the button's tooltip. The label pointed the operator at the wrong node. groupState now makes ONE ordered decision and returns the label, its colour and the reason together, so the tag can never describe a different condition than the one disabling the button. A group held up by a node that references the same address but cannot be adopted with it reads "blocked by peer"; two MASTERs is distinguished from none. Display only: the endpoint's checks and refusals are untouched. Backend suite: 1366 passed, 152 skipped. Frontend build clean.v1.10.11 |
||
|
taylanbakircioglu
|
4f24d5bdd9 |
fix(vip): list adoption blockers once per instance (v1.10.10)
Since the panel groups a VRRP instance into one row, the blocker lists of all its members are merged - and every node reports the SAME problems about the SAME shared config. A two-node pair therefore showed each issue twice, in the Adoptable tooltip and in the adopt dialog. Plain de-duplication does not collapse them because the two files report different line numbers for the same directive. mergeBlockers keys on the message with a leading "line N:" stripped and keeps the first occurrence, so each distinct problem appears once while the text the operator reads still carries a line reference. Display only: the endpoint already evaluated the combined set across every node, and what it accepts or refuses is unchanged. Backend suite: 1366 passed, 152 skipped. Frontend build clean. |
||
|
taylanbakircioglu
|
a87994e06a |
fix(vip): refuse adoption that strands a node or normalises a peer (v1.10.9)
Three findings from a second pass over the adoption flow, all of the same class: something real leaving the set silently. 1. STRANDING. _collect_instance_participants can only match a node it can READ, that is ENABLED, and that is in the SAME pool. Each of those is a door a genuine member of the VRRP group leaves through without a word, and the nodes that remain are rewritten while it keeps serving the same address from an unmanaged config. Found on a live pool: one node of a pair had an unclosed vrrp_instance block, so it parsed to nothing while its partner parsed cleanly. Rather than guard each door, ask the question directly: does any reported keepalived.conf mention THIS virtual address without being one of the nodes we are about to adopt? Refuses naming the node and the reason. Scoped on the address so an unrelated file elsewhere cannot block every adoption, and excluding nodes already under management (a standing VIP, or our ownership marker) because those are not stranded. 2. SILENT NORMALISATION. prefix_length, unicast/multicast mode, HAProxy tracking and the VRRP password are stored ONCE on the VIP and re-rendered onto EVERY member, so whichever node was clicked imposed its settings on the others. prefix_length is the sharpest: the design refuses to GUESS a netmask for a live VIP, and copying one node's netmask onto another is that same change wearing a different hat. All four must now agree, with both values named in the refusal. The VRRP secret is compared by decrypting each node's token - Fernet is non-deterministic, so ciphertexts cannot be compared - and a token that will not decrypt is an error rather than an assumed match. 3. THE TAKEOVER AUTHORISATION WAS NOT ONE-SHOT. takeover_expected_hash is the permission to overwrite a keepalived.conf that lacks our ownership marker. It was written at adoption and never cleared, so it stayed valid for that file content indefinitely: restoring the pre-adoption file would have been overwritten again with no fresh human approval. It is now retired when a member acknowledges our rendered config, gated on the acked hash matching applied_config_hash so a partial or failed deploy never drops it and leaves the VIP unable to converge. The panel applies the stranding rule too, so the Adopt button is disabled with the reason instead of letting the operator click into a 422. No schema change, no agent change, no API-shape break. Backend suite: 1366 passed, 152 skipped. Frontend build clean. |
||
|
taylanbakircioglu
|
7d95c737f0 |
fix(vip): adopt the whole VRRP instance, not one node (v1.10.8)
Four defects found while tracing the adoption flow end to end after v1.10.4
reached a live HA pair.
B3/B4 (one root, one fix). Adoption took only the node whose row was clicked:
- adopting the BACKUP alone produced a VIP that apply always rejects, because
apply requires exactly one MASTER member;
- adopting the MASTER alone left the peer unmanaged, and adopting it
afterwards hit the VRID-collision guard with 409, so a pair could never be
completed from the panel;
- on a UNICAST instance the single-member render dropped the unicast block
entirely (render_keepalived_conf emits it only when peer_ips is non-empty),
so keepalived fell back to multicast on the adopted node while its peer
stayed unicast. They stop seeing each other and BOTH claim the VIP.
Adoption now resolves the whole instance via _collect_instance_participants,
keyed on (virtual_router_id, virtual address) - the same key keepalived uses to
group nodes. Each participant becomes a member with the role, priority and
interface its own file declares, and its own one-shot takeover hash, so the
per-node overwrite guard is unchanged. It refuses, naming the reason, when the
group has other than one MASTER, when advert_int differs across nodes, when a
declared unicast peer is not among the nodes being adopted, or when a node is
already in a live VIP - the one-active-VIP-per-agent rule that create/update
enforce via _validate_members_against_pool and adoption never called.
B1. The Apply Management "View Change" regex matched vip-(create|update|delete)
only, so an adopt version fell through to the generic HAProxy diff and rendered
the cluster's whole haproxy.cfg as removed. Display-only, but alarming. A test
now asserts every action _stage_vip_version can stage is in that alternation.
B2. Rejecting an adoption hid the node from the panel permanently:
vip_discoveries.adopted_vip_id is write-once, a VIP is only ever soft-deleted so
the column's ON DELETE SET NULL never fires, and the agent does not re-report a
file whose hash has not changed. Rather than clearing the column on each path,
adoptability is derived from whether the linked VIP is still active, which
self-heals reject, undo-reject and approved teardown alike.
The panel now lists one row per instance instead of per node, and the adopt
dialog names every node that will be taken over. Blockers are aggregated across
all of them, matching what the endpoint checks.
No schema change, no agent change, no API-shape break: /api/vip/discoveries
gains a derived field and /api/vip/adopt keeps its request body.
Backend suite: 1359 passed, 152 skipped. Frontend build clean (no new lint
warnings in VIPManagement.js).
|
||
|
taylanbakircioglu
|
eda7f36c93 |
fix(vip): scope the HA/VIP page to the selected cluster (v1.10.7)
The page ignored the cluster picker in the header. Both the VIP table and the v1.10.4 "Unmanaged keepalived detected" panel queried the whole fleet, so on an install with more than one cluster the lists showed every cluster's nodes at once and did not change when the selection did - the panel looked stuck on one cluster's keepalived. - GET /api/vip/discoveries takes the same optional cluster_id the VIP list already took, mapped to the cluster's pool via haproxy_clusters exactly like list_vips does. Omitting it still returns the whole fleet, so no existing caller changes behaviour. - VIPManagement reads selectedCluster from ClusterContext (it only took the cluster list before) and sends cluster_id on both fetches. The fetch callbacks depend on the scope, so switching cluster refetches instead of showing stale rows. - tests/test_vip_discoveries_scope.py pins both defects this endpoint has had: that the route reaches its own handler rather than being parsed as a vip_id (asserting != 422 specifically, since the repo's generic endpoint-auth tests accept 422 alongside 401/403 and therefore could not catch it), and that the cluster filter resolves cluster -> pool and short-circuits when absent. Behaviour change worth calling out: the VIP table is now scoped to the selected cluster where it was fleet-wide before. The API still serves the fleet-wide view to any caller that omits cluster_id. Backend suite: 1345 passed, 152 skipped. Frontend production build clean. |
||
|
taylanbakircioglu
|
11e5bf57d9 |
fix(vip): make the adoption panel reachable again (v1.10.6)
Follow-up to #58. The "Unmanaged keepalived detected" panel it shipped never appeared on any deployment. GET /discoveries was declared after GET /{vip_id} in routers/vip.py, and FastAPI matches routes in declaration order, so every request for the discovery list was routed into get_vip, which takes vip_id: int and rejected "discoveries" with 422 before list_vip_discoveries ever ran. The failure was completely silent. The agents reported their configs correctly, the rows landed in vip_discoveries, and the HA/VIP page treats any non-OK response as "nothing to show" - so the feature was invisible with no error in any log. Confirmed against a real fleet: two discovery rows present in the database, one with a parsed candidate, and an empty panel. - move list_vip_discoveries above the /{vip_id} routes, with a comment stating the ordering requirement - add tests/test_router_path_shadowing.py: a static source scan that fails if any literal path in any router is declared after a parameterised route that would swallow it. The whole router tree is clean; the detector is itself tested against the pre-fix ordering so the guard cannot pass vacuously. POST /adopt is unaffected: no POST /{vip_id} route exists. Also carries the release metadata for #58 (v1.10.4) and #60 (v1.10.5), which are published together with this fix rather than as separate artifacts. No schema, API-shape, frontend or agent change. Data reported under the earlier code is not lost - existing rows show up as soon as this backend is deployed, with no agent action needed. |
||
|
taylanbakircioglu
|
a4c74f2a52 |
Merge pull request #60 from mustafaulukaya/fix/acme-http01-split-deployment
HTTP-01 challenge backend on split deployments |
||
|
taylanbakircioglu
|
a36dd87a74 |
fix(agent): port VIP adoption into the in-script daemon so self-upgraded agents get it
Found during an impact analysis of the agent-script change in PR #58. linux_install.sh contains TWO daemon implementations and which one runs depends on how the agent reached its current state: - Fresh install: the heredoc at lines 923-2646 is written to /usr/local/bin/haproxy-agent and systemd runs that file. - Self-upgrade: perform_agent_upgrade copies the downloaded INSTALLER script over that same path (`cp "$temp_script" "$current_script"` with current_script=/usr/local/bin/haproxy-agent). systemd then runs the installer with `daemon` + SKIP_TO_DAEMON=true, which takes the separate in-script daemon that lives after the heredoc. PR #58 added _kp_discover and the one-shot takeover only to the heredoc copy, so both were absent from the path that self-upgraded agents actually run — and self-upgrade is exactly the path the release notes tell operators to rely on ("nodes will pull the new script through the normal agent-upgrade path"). The feature would have worked on a freshly installed node and been silently inert on every upgraded one. The file's own banner warns about this ("check_agent_upgrade() - Multiple locations ... TIP: Search for function name to find all occurrences!"), and send_heartbeat / fetch_and_deploy_keepalived_config / get_haproxy_stats_csv are already maintained as parallel copies for the same reason. Verified empirically rather than by reading: the script was instrumented and run in a container exactly as systemd invokes it after an upgrade (SKIP_TO_DAEMON=true, `bash linux_install.sh daemon`), inspecting the live `declare -f fetch_and_deploy_keepalived_config`. before: DISCOVERY_YOK TAKEOVER_YOK after: DISCOVERY_VAR TAKEOVER_VAR ENDPOINT_VAR Both blocks are ported verbatim from the heredoc copy with indentation adjusted; no logic changed, so the guard semantics are identical in both paths — takeover still requires allow_takeover AND a non-empty expected hash AND a matching on-disk md5, and anything else falls through to the existing "externally managed — refusing to overwrite" branch. `bash -n` passes. Backend suite unchanged at 1263 passed. |
||
|
taylanbakircioglu
|
2f125da043 |
Merge pull request #58 from mustafaulukaya/feature/keepalived-vip-adoption
Adopt an existing keepalived VIP (Issue #27 follow-up) |
||
|
Mustafa ULUKAYA
|
9e6e4dd03b |
fix(acme): repair the diagnostics contract the GET probe broke
Running the suite properly — the previous rounds could not, pytest was not installed locally — surfaced four failures, all in the code this branch touches. main is clean at 1410 passed, so these were mine. Two were real defects, not stale expectations: check_port80 turned an unreadable body into a hard failure. The GET probe read the body outside any guard, so a connection reset mid-response, or a server that hangs after headers, made a healthy 404 fail. The body is EVIDENCE, not a precondition: when it cannot be read the check now falls back to the status-only semantics it has always had, and records body_class 'unread'. The stricter rule applies only when there is something to judge. check_routing raised KeyError on a row without `acme_enabled`. The column gates a warning, so a row shape lacking it should not take down the whole diagnostic. The rest were expectations that had to change, because the contract did: - The probe is a GET now, so the test doubles needed a body. `_FakeHEADResp` became `_FakeGETResp` with headers and a readable content stream. - "A port-80 frontend row exists" no longer means ok. That assertion is exactly the bug: it describes what the database wants, while the nodes run whatever was last applied — which is how this check reported success throughout an incident where the live config had no usable challenge route. New coverage for the branches that had none, including the case that started all of this: a proxy that has lost its /.well-known/acme-challenge/ location serves its SPA with HTTP 200, which the old `status in (200, 404)` rule accepted as healthy. Also the tcp-only cluster, which must warn rather than fail — SiteWizard blocks submit on any failing check, so failing there would lock those installs on upgrade day. Verified against real dependencies rather than a stub harness: the app boots with the challenge route registered; the field validator fires through pydantic on both cluster models, normalising whitespace and rejecting scheme-less and loopback values; `model_fields_set` really does distinguish an explicit null from an omitted key, which is what makes clearing the field work; the settings validator rejects both raw and jsonb-encoded bad values; and a legacy scheme-less value is skipped so the next source renders. Suite: 1477 passed on the branch, 1410 on main, 0 failed on either. |
||
|
Mustafa ULUKAYA
|
47cc79dcf7 |
fix(acme): address review findings on the challenge-backend hardening
Five confirmed findings from an adversarial review of the branch, four of them regressions introduced by it. Fall through to the next source when a stored URL cannot be resolved. Stopping at the first non-empty candidate emitted a backend section with no `server` line: the section exists so `haproxy -c` passes and Apply succeeds, then every challenge request 503s from an empty backend with nothing to show for it. Scheme-less values are common — the settings field was free text until this branch — so this was reachable on real installs. Selection moved into `select_acme_backend_source()` so it is testable and the skipped candidates are logged rather than silently dropped. Report a challenge backend with no server line. `extract_acme_backend_target` returns None for that section, and the loopback filter skipped falsy targets, so the case above would have been reported as "challenge route present in applied config" — the new check confirming the very state it exists to catch. Do not narrow the row set feeding the routing check's `fail` branch. Adding a mode filter to the WHERE clause turned a tcp-only port-80 cluster from "ok" into "fail", and the site wizard blocks submit on any failing check, so those installs would have been locked on upgrade day. Mode is now examined in Python and only downgrades to `warn`, using an expression that is character-for- character the renderer's normalisation. Match the agent's config selector. The applied-config lookup omitted `is_active = TRUE`, so it could read a superseded row and report on a config the nodes never received. Extraction now happens in SQL rather than pulling whole configs — these run to hundreds of KB. Select `acme_backend_url` when loading the existing cluster. It was absent, so the entity snapshot recorded old_values as NULL unconditionally and rejecting the pending version wiped the operator's per-cluster URL back to the global loopback default — re-creating the exact failure this branch removes. Also carry `acme_enabled` and `acme_backend_url` through cluster creation. The create model declared neither and the INSERT wrote neither, so a cluster created with ACME switched on came back switched off with no error shown. Refuted and deliberately not changed: settings PUT re-validating a stored loopback value (it validates only what is submitted), an apply-path connection leak (the 422 propagates to a handler that closes it), and the modal discarding backend rejection reasons (the envelope matches). |
||
|
Mustafa ULUKAYA
|
bb774141d4 |
fix(acme): make the challenge backend fixable from the panel
Correcting a wrong ACME challenge backend was impossible without a shell, and
even with one the correction did not reach the nodes.
The mint gate only fired when `acme_enabled` flipped. `acme_backend_url` was
written to the DB and minted nothing, so Apply answered "No pending changes to
apply" and the nodes kept the old address forever. It is now decided by
comparing the rendered `server _acme_mgmt` line against the active version —
the one line that answers "would the nodes talk to a different address?".
Comparing whole configs would flag every unrelated pending edit.
The field had no UI at all. Added to the cluster form with validation that
mirrors the backend rules, and keyed on `model_fields_set` so clearing it
reverts to the global setting — with a plain `is not None` test an empty box is
indistinguishable from "not submitted", so a value could never be removed.
Validation is asymmetric on purpose (utils/acme_backend_url):
- at the write boundary, reject what cannot express a reachable target —
including the two silent traps: a scheme-less value became `localhost`, and
an out-of-range port raised inside the generator and destroyed the config
- at render time, never reject. The shipped defaults are themselves loopback,
so refusing to render would make every acme_enabled cluster unappliable,
including for changes unrelated to ACME. Problems are logged and surfaced.
The port-less default stays 8080 rather than moving to HTTP's 80: the bundled
compose publishes nginx on 8080, so installs relying on it work today and the
first sign of breaking them would be the unattended renewal loop months later.
The omission is warned about instead.
RFC1918 is allowed and is usually the right answer here, and no DNS resolution
is performed — both deliberate departures from utils/ssrf_guard, whose policy
is the opposite of what this address needs. What the management host can
resolve says nothing about what the HAProxy node can reach.
Diagnostics stop reporting success on a dead path:
- check_port80 uses GET instead of HEAD and classifies the body. A proxy that
has lost its /.well-known/acme-challenge/ location serves its SPA with HTTP
200, which `status in (200, 404)` accepted as healthy. Warnings also surface
when other domains pass, which previously hid the most diagnostic outcome.
- check_routing filters `mode`, joins `acme_enabled` and reads the APPLIED
config instead of counting database rows, and reports a loopback target.
- every new condition is `warn`, never `fail`: the site wizard blocks submit on
any fail, so a new failing condition would lock every install on upgrade day.
Also: normalise `frontends.mode` once per frontend. It is nullable, and the
raw value was interpolated into `mode {}`, emitting a literal `mode None` that
HAProxy rejects — taking down the whole cluster config. The ACME gate and the
backend-mode check now read the same normalised value.
And stop hardcoding PUBLIC_URL / MANAGEMENT_BASE_URL in docker-compose, which
silently ignored the operator's .env and made the wrong default load-bearing.
|
||
|
Mustafa ULUKAYA
|
acfd32dd63 |
fix(acme): stop shipping config-generation failures as applied config
`generate_haproxy_config_for_cluster` reports failure by RETURNING a one-line
comment ("# Error generating configuration: ...") rather than raising. Nothing
in the backend checked for it, so the apply path hashed that comment, stored it
as an APPLIED config version and pushed it to every agent — silently replacing
a cluster's entire haproxy.cfg.
Any exception inside the generator triggers this. An out-of-range port in
`acme_backend_url` is enough: `urlparse('http://h:99999').port` raises
ValueError, the outer `except` swallows it, and the cluster loses its config.
- add `is_config_generation_error()` next to the sentinel definitions so call
sites stop matching the string by hand
- guard the apply path: refuse with 422 and leave the running config in force
- guard the ACME-toggle PENDING mint, and let HTTPException through the local
`except Exception`, which would otherwise report success while no pending
version exists
Also make the challenge backend diagnosable without shell access, since this
block previously emitted no log line at all:
- log the rendered `host:port` and WHICH source chose it (cluster override,
system setting, or the MANAGEMENT_BASE_URL fallback) under the greppable
`ACME-BACKEND` keyword
- warn when the rendered address is loopback: HAProxy resolves it on the node,
not on the management host, so it can only work on an all-in-one install —
and it is exactly what the shipped defaults produce
- log peer, X-Forwarded-For and Host on the challenge endpoint, which answers
"did the request arrive at all?" — the question that separates a wrong
backend address from a wrong response body
Refs the HTTP-01 investigation: a split deployment rendered
`server _acme_mgmt <mgmt>:8080` against a port with no listener, and every
existing check reported success.
|
||
|
mustafa.ulukaya
|
ef26860df9 |
feat(logging): unified request/response log with configurable retention (v1.11.0)
Until now the only record of what happened was `user_activity_logs`, which stores non-GET 2xx operations with no bodies. When something failed you could see that a counter went up, never what was sent or what came back. This adds one queryable timeline covering both directions: - inbound: every API call, including GETs and including 4xx/5xx, with the user, client IP, status, duration and — redacted, size-capped — the request and response bodies. - outbound: every 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 underneath. Implementation notes: - Capture is a pure-ASGI middleware that TEES the request and response streams rather than draining them. `await request.body()` inside a BaseHTTPMiddleware would consume the receive channel and break the raw-body agent heartbeat handler. Registered last so it is outermost: it then sees the final client-visible response and seeds correlation_id_context before the error handler reads it. - Rows are written by a batching background writer with a bounded queue, so the request path never awaits the database and a saturated logger drops rows visibly (surfaced on the page) instead of blocking. Redaction runs on the writer, off the request coroutine. - Secrets never land: headers are an allowlist with Authorization/Cookie kept only as a presence marker; body keys and value shapes are redacted (passwords, tokens, api_token, API keys, private-key PEMs, JWTs); the ACME JWS request body is never stored, because a stored protected+signature pair is a replayable credential — a summary is logged instead; DNS-provider errors record only the exception type; 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 (7 / 30) plus a hard row cap (500k), whichever is reached first. Pruned in batches under a Postgres advisory lock, with the day counts bound as parameters, never interpolated. - New permissions requestlog.read / requestlog.manage. super_admin and security_admin get both, operator gets read, viewer gets neither. Schema: one new table (request_logs) plus its settings seed, SCHEMA_VERSION 10 -> 11, auto-migrated. No existing table altered, no agent or rendered-config change. Kill switches: REQUEST_LOG_ENABLED=false (middleware never registered) or the `enabled` toggle in Settings. Tests: 245 new (7 backend files + 1 frontend), full suite 1655 backend + 17 frontend passing. |
||
|
mustafa.ulukaya
|
78af849fdc |
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). |
||
|
mustafa.ulukaya
|
709817fec3 | chore(version): bump to 1.10.4 - adopt an existing keepalived VIP | ||
|
mustafa.ulukaya
|
822c441d34 |
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. |
||
|
mustafa.ulukaya
|
1ca811e211 |
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. |
||
|
mustafa.ulukaya
|
164841219a |
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. |
||
|
mustafa.ulukaya
|
d92a7e9660 |
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.
|
||
|
mustafa.ulukaya
|
7dfd31832a |
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. |
||
|
mustafa.ulukaya
|
8ac567dfe0 |
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. |
||
|
taylanbakircioglu
|
02667bbda4 |
test(acme): make the wizard regression suite pass under the default jest timeout
Follow-up to #57. The contributed tests render the whole ACMEAutomation tree (antd Steps + Form + Select) and drive it through all three wizard steps, which takes 4-9 seconds per test. Under jest's default 5s per-test limit two of them failed, so `npm test` did not pass as shipped: ✕ an explicitly picked HTTP-01 account survives the step change and is what gets submitted -> Exceeded timeout of 5000 ms ✕ the wildcard guard still applies on the Review step, where Submit lives -> Exceeded timeout of 5000 ms The PR's reported 5/5 holds only when the runner is invoked with an explicit --testTimeout. Setting it in the file instead means the suite passes however it is invoked, which matters because the frontend image build runs `npm run build` and never the tests, so nothing else would have caught this. Verified with the default runner (no flags) after the change: 5/5 pass. Test-only. No production code touched.v1.10.3 |
||
|
Taylan Bakırcıoğlu
|
c97df53da8 |
Merge pull request #57 from appouse/feature/multiple-account-acme-automation
Multi-account ACME: the certificate wizard honours the selected account (v1.10.3). Verified before merge. The contributed regression tests were run against the PRE-FIX component to confirm they actually catch the bug: 4 failed / 1 passed, reproducing the reported symptom exactly (Expected "http-01" / Received "dns-01", Review rendering the default account's email instead of the picked one, and the wildcard warning absent). With the fix: 5/5 pass. The three root causes were each confirmed against the tree — rc-field-form's useWatch honours options.preserve (es/useWatch.js:66), the backend default is ORDER BY created_at DESC (routers/letsencrypt.py:505) while the list is served ORDER BY id (:213), and the null-dns_provider normalization matches the backend's own at :473. Frontend production build succeeds; backend suite unchanged at 1243 passed. |
||
|
mustafa.ulukaya
|
be01ddd119 |
test(acme): drive the certificate wizard to pin multi-account account resolution
Renders the real component and walks it through all three wizard steps, because the bug these cover was invisible to any unit test: it only appeared once the wizard advanced PAST the step that owns the account Select, since Form.useWatch reports only currently-rendered fields. Assertions describe behaviour rather than markup - the primary evidence is the POST body (account_id paired with challenge_type), compared against a fixture whose DNS-01 account is deliberately both the lower id and the older account, which is the exact shape that made the UI default and the backend default disagree. Verified by running the suite against the pre-fix component: the picked-account test reports challenge_type "dns-01" where "http-01" is expected, the Review test shows "Active (dns@example.com) / Challenge Method: DNS-01 / DNS provider: godaddy" for a chosen HTTP-01 account - the reported symptom reproduced - and the wildcard-guard test finds Submit enabled. Four fail, one passes: the DNS-01 selection path, kept as a positive control because it worked before (the default the wizard fell back to happened to be the DNS-01 account) and must keep working after. Adds src/setupTests.js with the ResizeObserver and matchMedia polyfills jsdom lacks and Ant Design 5 needs before any Select can open. |
||
|
mustafa.ulukaya
|
bbd8359f50 |
docs(v1.10.3): document the multi-account ACME wizard fix
Release note covering the three compounding faults, and upgrade notes stating that this is frontend-only with nothing to do on upgrade. Two points are called out for operators rather than glossed: installations that never picked an account explicitly were already using the newest valid account, so only the preview was wrong; and the wildcard guard that stopped applying on Review was a lost warning, not a correctness hole, since the backend still rejected those requests. |
||
|
mustafa.ulukaya
|
dd7b7822bf | chore(version): bump to 1.10.3 - multi-account ACME wizard fix | ||
|
mustafa.ulukaya
|
c4139bb11a |
fix(acme): honour the selected ACME account in the certificate wizard
With more than one account registered, picking an HTTP-01 account in Request ACME Certificate still submitted a DNS-01 request, which the API rejected with "The selected ACME account has no DNS provider configured for DNS-01." Three faults compounded: 1. Form.useWatch reports only fields that are currently rendered. The account Select lives on the Configuration step, so the moment the wizard advanced to Review the watch read undefined and the wizard fell back to the default account - even though the value was still in the form store. Both wizard watches now pass preserve: true. The same fault silently 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 first valid entry of a list ordered by id, i.e. the oldest - the opposite account whenever the two differ. The wizard now resolves the same account 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. Both are now derived from one resolved account, so the pair can no longer describe two different accounts. Also: the Review step showed the default account's address instead of the chosen one, and Submit stayed enabled when the resolved account was deactivated (the fallback can land on a non-valid account, and the Select lists deactivated accounts). Single-account installations are unaffected. |
||
|
taylanbakircioglu
|
dbb9189f16 |
fix(ui): make Apply Management readable in dark mode (v1.10.2)
Three dark-mode defects reported on the Apply Management page, all the same
class of bug: light-mode colour literals hardcoded where theme tokens belong.
1. The "Pending Changes" box was painted background #fffbe6 with border
#ffe58f. In dark mode the text on top is light, so the version name,
timestamp and "View Change" link sat on a cream panel and were unreadable.
Measured contrast was 1.03:1; it is now 11.50:1 (secondary text 1.03:1 ->
7.02:1).
2. The added/removed rows in the View Change diff used #f6ffed/#52c41a and
#fff2f0/#ff4d4f, which stayed near-white inside the otherwise dark diff
panel. Now 5.49:1 (added) and 4.01:1 (removed), from 2.21:1 and 2.99:1.
3. The "Apply All Configuration Changes" confirm dialog came up white. This one
is not a colour literal: 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 fall back to the light algorithm. Registering
ConfigProvider.config({ holderRender }) once at the app root wraps that
detached root in the same ConfigProvider. Verified against the installed antd
5.29.3 source rather than assumed: config-provider/index.js sets
globalHolderRender, and modal/confirm.js wraps the dialog with it. This fixes
EVERY static dialog in the application — 12 components call Modal.confirm —
not just this page.
While in the file, six more instances of the same bug were fixed: the error
Alert border, the VIP pending-delete row, two ACME/pending version panels, the
applied-version panel and the agent-error recommendation box.
Light mode is byte-identical. Each token resolves under the default algorithm
to exactly the literal it replaced (colorWarningBg -> #fffbe6, colorSuccessBg ->
#f6ffed, colorErrorBg -> #fff2f0, colorInfoBg, colorErrorBorder, ...), so this
release can only change dark mode. Contrast was measured by resolving the real
design tokens under both algorithms and computing WCAG ratios, not by eye.
Note the diff rows were already low-contrast in LIGHT mode (2.21:1 and 2.99:1)
and remain so; that is the design system's own success/error pair and changing
it would alter the established light-mode appearance, so it is left alone.
holderRender is registered in an effect rather than during render, since
ConfigProvider.config() mutates antd module state; effects still run long
before a user can click anything that opens a static dialog.
Frontend only: no schema, no SCHEMA_VERSION bump, no API change, no environment
variable, zero agent impact. Backend suite unchanged at 1243 passed.
v1.10.2
|
||
|
taylanbakircioglu
|
eee0a4716a |
feat(ssl): encrypt the pending CSR private key at rest (v1.10.1, closes #53)
Closes the follow-up filed during the v1.9.0 CSR review. The private key of a
PENDING CSR is now Fernet-encrypted in the database instead of being stored as
a raw PEM.
Why this key specifically: it is the one key in the system that sits idle. It
is generated at CSR creation, waits for an external CA to sign the request
(days to weeks), and is destroyed the moment the signed certificate is
imported. It is never transmitted to an agent and never leaves the server.
ssl_certificates.private_key_content and the ACME order keys are deliberately
NOT covered, because agents must receive those in plaintext on every poll, so
encrypting them at rest buys nothing without an end-to-end redesign.
Implementation follows the pattern already used for the VRRP secret, TOTP
secrets and DNS provider credentials: a new utils/csr_key_crypto.py with its
own CSR_ENCRYPTION_KEY env var and its own HKDF info string
("csr-private-key-v1"), so rotating one secret class never affects another.
No schema change and deliberately NO SCHEMA_VERSION bump: the Fernet token
replaces the PEM inside the existing ssl_csrs.private_key_pem TEXT column. A
bump would re-run the migration sequence and re-seed the four built-in roles to
their defaults, which is a needless side effect for a storage-format change.
Backward compatible with no data migration. Rows written before this release
hold a raw PEM and are still read unchanged; the discriminator is exact rather
than a heuristic, since a Fernet token is base64url and can never contain the
"-----BEGIN" marker. Legacy rows drain naturally because a CSR's key copy is
NULLed on import.
A key that cannot be decrypted (SECRET_KEY rotated while CSR_ENCRYPTION_KEY was
unset) now fails with an explicit "delete this CSR and create a new one" error.
Previously that situation would have surfaced as the far more confusing
"certificate does not match this CSR's private key".
Also documents all four per-purpose encryption keys in .env.template. Only
VIP_ENCRYPTION_KEY was listed; MFA_ENCRYPTION_KEY and
DNS_PROVIDER_ENCRYPTION_KEY had been missing since v1.6.0 and v1.8.0.
Verified before release, on a corporate pre-production environment and locally:
- Full backend suite 1234 -> 1243 passed (+9 new tests), 0 failed.
- Against a real Postgres: a CSR created through the API stores a Fernet token
with no PEM header in the column, and imports successfully.
- Full 1.10.0 -> 1.10.1 -> 1.10.0 drill on one database volume. The upgrade
logs "Schema already at version 10 (>= 10); skipping migration run", so no
migration executes and the built-in roles are not re-seeded. A CSR created on
1.10.0 with a plaintext key imports successfully after the upgrade, which is
the backward-compatibility guarantee proven against a real row rather than a
mock.
- rsa-2048, rsa-4096 and ecdsa-p384 all round-trip through create, encrypt,
decrypt and import.
- Key derivation is stable across processes: two independent containers sharing
SECRET_KEY decrypt each other's tokens (required for UVICORN_WORKERS > 1 and
multi-replica deployments), while a different SECRET_KEY yields None rather
than a wrong key or an exception.
- Downgrade behaviour was measured, not assumed: 1.10.0 cannot parse the token
and fails with HTTP 500 "key parse failed (encrypted?)" rather than pairing a
wrong key. The rollback note states the measured behaviour.
- No CSR endpoint returns the key in any form: list and detail responses
contain neither a PEM nor a Fernet token.
Not changed here, from the issue's "worth folding in" list: the create rate
limit is not a concurrency guard, create_csr holds a pooled connection across
RSA key generation, detail=str(e) echoes internal error text (a repo-wide
convention), and is_global skips cluster validation in both routers/ssl.py and
routers/csr.py. None are storage concerns and each is a separate change.
v1.10.1
|
||
|
Taylan Bakırcıoğlu
|
3c8832330a |
Merge pull request #54 from appouse/feature/godaddy-dns-provider
GoDaddy DNS-01 provider for ACME (v1.10.0). Validated on a corporate pre-production environment before merge: backend suite 1221 to 1234 passed (+13, exactly the new GoDaddy tests) with 0 failures; a wire-level harness against a fake GoDaddy API confirmed the apex+wildcard pair coexists, removing the last value uses DELETE rather than PUT [], an unreadable read fails closed with no write attempted, and across every scenario not one request reached a zone-wide endpoint (SPF, DKIM and DMARC survived untouched). The path-guard premise was measured directly: with yarl 1.24.5 a '.' segment normalizes onto the zone-wide TXT endpoint and '..' onto the whole-zone endpoint, so the guard in _rrset_path is load-bearing. No schema, environment, frontend or agent change. Closes #55v1.10.0 |
||
|
mustafa.ulukaya
|
81ab674072 |
docs(v1.10.0): document the GoDaddy DNS provider and upgrade notes
README: add GoDaddy to the two feature bullets and to the DNS-01 provider catalog, spelling out that the API Key must be a Production key (the first key the developer dashboard issues is an OTE/test key and is rejected), that the zone must be in the same account, that the account needs a registered domain before GoDaddy permits DNS API access, and that a Personal Access Token works with the Secret left blank. Note that publishing is automatic for GoDaddy as well as Cloudflare, and add the release-notes entry. UPGRADE_GUIDE: new section stating there is no SCHEMA_VERSION bump, so the built-in-role re-seed warning from v1.9.0 does not apply, and no new environment variable, API-shape or agent change. Two limits are stated explicitly rather than glossed: the credential check is a read, so a token with read but not write scope saves successfully and only fails at the first publish; and downgrading after adopting GoDaddy is not a no-op, because an unknown provider name degrades DNS-01 orders to the manual-confirm path and leaves published TXT records marked cleaned without being removed. |
||
|
mustafa.ulukaya
|
6bf6d016f5 | chore(version): bump to 1.10.0 - GoDaddy DNS-01 provider | ||
|
mustafa.ulukaya
|
0a0226c758 |
test(dns): cover GoDaddy relative-name derivation, RRset merge and request handling
Twelve tests, no network and no database, in the existing pure-logic style. The merge helpers are covered directly (additive add, idempotent republish, tombstone filtering, remove-one-of-many, remove-the-last-value signalling DELETE), but helper math alone would stay green if the write path stopped using it, so add_txt_record and remove_txt_record are also driven against a recording stub: the assertions pin that a sibling value survives a publish, that an already-published value issues no write, that an unreadable read raises instead of replacing the set, that removing the last value emits DELETE and never an empty PUT, and that no call is ever aimed at a zone-wide path. _request is exercised through a fake response for the cases that only appear against the real API: an empty 204 body must not raise, a 3xx must not read as success (redirects are not followed), a transport failure mid-read must not be mistaken for an empty body, and each error status must produce a message naming what the operator has to fix. Also covers the auth header in both forms, that the sanitizer strips credentials from composed error text, that the module does not log at all, the credential-field schema against the upsert validator's own key and length rules, and the two-key encryption round trip. Verified by mutation: nine deliberate breakages of the provider - single-value PUT, empty PUT instead of DELETE, coercing an unreadable read to empty, treating 3xx as success, following redirects, swallowing transport errors, dropping the dot-segment guard, lowering the TTL below the API floor, and removing sanitization - are each caught by at least one test. |
||
|
mustafa.ulukaya
|
8e534ef170 |
feat(dns): add GoDaddy DNS-01 provider (API Key+Secret / PAT, additive RRset writes)
Registers a third pluggable DNS provider for ACME DNS-01 alongside Manual and Cloudflare. Credentials are an API Key + Secret pair; leaving the Secret blank sends the Key as a Personal Access Token (Bearer), which is the migration path as GoDaddy retires the sso-key scheme. GoDaddy's Domains API v1 has no per-value TXT write: PUT on a record set replaces every value at that name. A certificate covering example.com and *.example.com publishes two different TXT values at the same _acme-challenge.example.com, so add/remove are read-modify-write - read the current set, merge, put the whole list back - with empty-data tombstone rows filtered out (they are rejected on echo) and DELETE used for the last value, since PUT with an empty array is rejected. The zone-wide sibling endpoints (.../records/TXT and .../records) would wipe SPF/DKIM/DMARC and the whole zone respectively, so the record path is built in one place that refuses an empty or dot segment. An unreadable record-set read fails closed rather than being treated as an empty set, because the PUT that follows would otherwise destroy the coexisting values. Zone lookup walks name suffixes probing the records API rather than the domain listing, so zones delegated to GoDaddy nameservers resolve and accounts that are rejected from the domain-details endpoint still work. Credential and eligibility failures during the walk surface instead of being reported as "no managed domain". Provider errors are sanitized at the single point where GoDaddy-supplied text enters a message, since those strings are persisted to order events and shown in the UI. No new dependency, no schema change, no frontend change - the credential form is rendered from the provider schema. |