RepairDKIMSigningConfig discards the key regeneration error and restarts rspamd once per key (caeeb629)
Found while evaluating dev for a downstream fork. Three points in caeeb629 ("Fix #117: regenerate missing DKIM keys in RepairDKIMSigningConfig"), all in core/internal/service/domains/domains.go.
1. The regeneration error is discarded
domains.go:937:
if !public.FileExists(keyPath) {
g.Log().Warningf(ctx, "DKIM key missing for %s/%s, regenerating", d.Domain, sel.name)
getDKIMRecordWithKeySize(d.Domain, sel.name, sel.size, false)
}getDKIMRecordWithKeySize returns (v1.DNSRecord, error) but is called as a bare statement, so both returns are dropped. Neither the compiler nor go vet flags this.
If regeneration fails, nothing is logged beyond the pre-emptive "regenerating" warning, and the loop still appends a signing block naming .../dkim/<domain>/<selector>.private — a file that does not exist. Rspamd then fails to sign for that domain with no indication of why.
2. rspamd is restarted once per regenerated key
getDKIMRecordWithKeySize does considerably more than the repair path needs: on the keys-missing path it rewrites dkim_signing.conf and restarts the rspamd container (domains.go:632).
RepairDKIMSigningConfig then overwrites that same file with the full rebuilt content and restarts rspamd itself at step 6 (domains.go:993).
So a host with N domains and missing keys — the clean-reinstall case #117 describes — restarts rspamd up to 2N times inside the loop before the caller does the real work and restarts once more.
Observed on a first boot of a clean stack: rspamd restarting repeatedly during startup, alongside
RepairDKIMSigningConfig failed: failed to change DKIM file permissions: lstat /opt/billionmail/rspamd-data/dkim: no such file or directory3. The key-existence check sits outside the mutex
In getDKIMRecordWithKeySize the !public.FileExists(...) check happens before mutex.Lock(), and generation then runs unconditionally. Two concurrent callers for the same domain and selector can therefore both run rspamadm dkim_keygen, the second overwriting the first key. Re-checking under the lock would close that.
Suggested shape
Extract a key-generation-only helper that creates the directory, runs dkim_keygen, writes the .pub file and fixes permissions — no signing-config write, no container restart — and have both getDKIMRecordWithKeySize and RepairDKIMSigningConfig call it. The repair loop can then report a failure and exclude that domain instead of emitting a block pointing at a missing key, and the redundant restarts disappear because the caller already restarts once at the end.
We have this applied downstream and are happy to open a PR if that is useful.
Source: Billionmail/BillionMail