fix(config+record): rollback zone files on checkzone failure; report rndc reload failures; fix named-checkconf root cause
Three classes of silent-failure bugs were leaving Web UI actions in
limbo without telling the operator:
1. record_add / record_delete:
- The file was already mutated + serial bumped before named-checkzone
ran. If checkzone failed, the user saw an error flash but the
half-applied record was on disk + serial had advanced. Re-running
would create duplicates.
- bind_reload() return value was discarded, so an rndc SERVFAIL was
silently logged as '添加成功'.
2. zone_create / zone_delete:
- Same bind_reload() silent-discard bug.
3. config_options_save / config_upstream_save:
- 'named-checkconf /tmp/named_check.tmp' treated the tmp file as
the master config, which refuses options-only directives
(0.0.0.0 / dnssec-validation / listen-on-v6) at the top level →
false-positive 'unknown option' validation failures that left
the user unable to save any options change.
- Auto-wrap logic re-wrapped already-valid options blocks, creating
'options { options { ... }; };' which named-checkconf rejected.
- No rollback: failures left the live file in whatever state the
tmp had mutated it to.
Fixes:
- append_record_to_file / delete_record_from_file now return the
original content; callers pass it to rollback_zone_file() on
checkzone failure.
- record_delete now runs named-checkzone (was missing entirely).
- All four write paths (record add/delete, zone create/delete)
surface rndc reload failures instead of always flashing '成功'.
- config_options_save and config_upstream_save now validate via
'named-checkconf' (whole config) after writing BIND_CONF_OPTIONS,
with .bak-based rollback on failure — no more /tmp/tmpfile tricks.
Verified end-to-end on prod: bad named.conf.options content triggers
HTTP 302 + '已恢复备份' flash, live file untouched, named-checkconf
remains OK; good content saves + reloads cleanly.
This commit is contained in:
@@ -450,51 +450,80 @@ def generate_zone_file(zone_name, records, soa=None):
|
||||
|
||||
def append_record_to_file(filepath, name, ttl, rtype, data):
|
||||
"""Append a new record to the zone file and bump the serial.
|
||||
|
||||
Returns (success, message).
|
||||
|
||||
Returns (success, message, original_content_for_rollback).
|
||||
The original_content can be passed to rollback_zone_file() if a
|
||||
subsequent named-checkzone / bind_reload step fails.
|
||||
"""
|
||||
try:
|
||||
with open(filepath) as f:
|
||||
content = f.read()
|
||||
original = f.read()
|
||||
except FileNotFoundError:
|
||||
return False, "Zone file not found"
|
||||
return False, "Zone file not found", None
|
||||
|
||||
# Build the new record line
|
||||
ttl_str = f'{ttl} ' if ttl else ''
|
||||
new_line = f'{name} {ttl_str}IN {rtype:<6} {data}\n'
|
||||
|
||||
# Append record at end of file
|
||||
content = content.rstrip('\n') + '\n' + new_line
|
||||
content = original.rstrip('\n') + '\n' + new_line
|
||||
|
||||
# Update serial
|
||||
content = bump_serial_in_content(content)
|
||||
|
||||
with open(filepath, 'w') as f:
|
||||
f.write(content)
|
||||
try:
|
||||
with open(filepath, 'w') as f:
|
||||
f.write(content)
|
||||
except OSError as e:
|
||||
return False, f"Write zone file failed: {e}", None
|
||||
run_cmd(f"chown bind:bind {filepath} 2>/dev/null || chown named:named {filepath} 2>/dev/null")
|
||||
run_cmd(f"chmod 644 {filepath}")
|
||||
return True, "OK"
|
||||
return True, "OK", original
|
||||
|
||||
|
||||
def rollback_zone_file(filepath, original_content):
|
||||
"""Restore a zone file to its previous content. Used when a record
|
||||
write succeeds but a downstream check (named-checkzone / rndc reload)
|
||||
fails — without this the user would see an error flash while the
|
||||
record is already on disk, leading to confusion on retry.
|
||||
"""
|
||||
if original_content is None:
|
||||
return
|
||||
try:
|
||||
with open(filepath, 'w') as f:
|
||||
f.write(original_content)
|
||||
run_cmd(f"chown bind:bind {filepath} 2>/dev/null || chown named:named {filepath} 2>/dev/null")
|
||||
run_cmd(f"chmod 644 {filepath}")
|
||||
except OSError:
|
||||
# Best-effort: if we can't restore, the operator must intervene.
|
||||
pass
|
||||
|
||||
|
||||
def delete_record_from_file(filepath, zone_name, idx):
|
||||
"""Delete the Nth record (0-indexed, excluding SOA) from the zone file.
|
||||
|
||||
Returns (success, deleted_record_str, message).
|
||||
|
||||
Returns (success, deleted_record_str, message, original_content_for_rollback).
|
||||
Caller should pass original_content to rollback_zone_file() if a
|
||||
subsequent named-checkzone / bind_reload step fails.
|
||||
"""
|
||||
soa, records = parse_zone_file(filepath)
|
||||
if idx < 0 or idx >= len(records):
|
||||
return False, "", "Record index out of range"
|
||||
return False, "", "Record index out of range", None
|
||||
|
||||
deleted = records[idx]
|
||||
|
||||
# Read raw lines and find the matching record line to remove
|
||||
with open(filepath) as f:
|
||||
raw_lines = f.readlines()
|
||||
try:
|
||||
with open(filepath) as f:
|
||||
original = f.read()
|
||||
raw_lines = original.splitlines(keepends=True)
|
||||
except FileNotFoundError:
|
||||
return False, "", "Zone file not found", None
|
||||
|
||||
# We need to find the Nth non-SOA, non-directive, non-comment, non-empty line
|
||||
# and remove it. This is tricky because the record might span lines.
|
||||
# Strategy: find the line that contains the record's name and type.
|
||||
|
||||
|
||||
# Build a search pattern for the record
|
||||
target_name = deleted["name"]
|
||||
target_type = deleted["type"]
|
||||
@@ -534,16 +563,27 @@ def delete_record_from_file(filepath, zone_name, idx):
|
||||
new_lines.append(line)
|
||||
|
||||
if not removed:
|
||||
return False, "", "Could not find record to delete"
|
||||
return False, "", "Could not find record to delete", None
|
||||
|
||||
content = ''.join(new_lines)
|
||||
content = bump_serial_in_content(content)
|
||||
|
||||
with open(filepath, 'w') as f:
|
||||
f.write(content)
|
||||
try:
|
||||
with open(filepath, 'w') as f:
|
||||
f.write(content)
|
||||
except OSError as e:
|
||||
return False, "", f"Write zone file failed: {e}", None
|
||||
run_cmd(f"chown bind:bind {filepath} 2>/dev/null || chown named:named {filepath} 2>/dev/null")
|
||||
run_cmd(f"chmod 644 {filepath}")
|
||||
return True, f'{deleted["name"]} {deleted["type"]} {deleted["data"]}', "OK"
|
||||
return True, deleted_str_for(deleted), "OK", original
|
||||
|
||||
|
||||
def deleted_str_for(deleted):
|
||||
"""Compact human-readable summary of a record for audit logs."""
|
||||
name = deleted.get("name", "")
|
||||
rtype = deleted.get("type", "")
|
||||
data = deleted.get("data", "")
|
||||
return f"{name} {rtype} {data}".strip()
|
||||
|
||||
|
||||
def bump_serial_in_content(content):
|
||||
@@ -798,11 +838,20 @@ def zone_create():
|
||||
write_named_conf_local(existing)
|
||||
|
||||
# Reload BIND
|
||||
bind_reload()
|
||||
reload_ok, reload_msg = bind_reload()
|
||||
|
||||
u = current_user()
|
||||
AuditLog.log(u.username, "创建Zone", f"域名: {zone_name}, 类型: {zone_type}")
|
||||
flash(f"域名 {zone_name} 创建成功", "success")
|
||||
if reload_ok:
|
||||
flash(f"域名 {zone_name} 创建成功", "success")
|
||||
else:
|
||||
# Files are on disk and named.conf.local is updated, but BIND
|
||||
# refused reload — surface it loudly so the operator doesn't
|
||||
# assume the zone is live.
|
||||
flash(
|
||||
f"域名 {zone_name} 配置已写入,但 BIND 重载失败: {reload_msg}",
|
||||
"error",
|
||||
)
|
||||
return redirect(url_for("zone_detail", zone_name=zone_name))
|
||||
|
||||
return render_template("zone_form.html")
|
||||
@@ -849,11 +898,17 @@ def zone_delete(zone_name):
|
||||
zones = [z for z in zones if z["name"] != zone_name]
|
||||
write_named_conf_local(zones)
|
||||
|
||||
bind_reload()
|
||||
reload_ok, reload_msg = bind_reload()
|
||||
|
||||
u = current_user()
|
||||
AuditLog.log(u.username, "删除Zone", f"域名: {zone_name}")
|
||||
flash(f"域名 {zone_name} 已删除", "success")
|
||||
if reload_ok:
|
||||
flash(f"域名 {zone_name} 已删除", "success")
|
||||
else:
|
||||
flash(
|
||||
f"域名 {zone_name} 已从配置移除,但 BIND 重载失败: {reload_msg}",
|
||||
"error",
|
||||
)
|
||||
return redirect(url_for("zone_list"))
|
||||
|
||||
|
||||
@@ -932,22 +987,36 @@ def record_add(zone_name):
|
||||
name = "@"
|
||||
|
||||
# Append record directly to zone file and bump serial
|
||||
ok, msg = append_record_to_file(zone["file"], name, ttl, rtype, data)
|
||||
ok, msg, original = append_record_to_file(zone["file"], name, ttl, rtype, data)
|
||||
if not ok:
|
||||
flash(f"添加记录失败: {msg}", "error")
|
||||
return redirect(url_for("zone_detail", zone_name=zone_name))
|
||||
|
||||
# Validate zone
|
||||
# Validate zone; on failure roll back the write so the user can retry cleanly
|
||||
# without leaving a half-applied record + bumped serial on disk.
|
||||
rc, out, err = run_cmd(f"named-checkzone {zone_name} {zone['file']}")
|
||||
if rc != 0:
|
||||
flash(f"Zone 验证失败: {out} {err}", "error")
|
||||
rollback_zone_file(zone["file"], original)
|
||||
flash(
|
||||
f"Zone 验证失败: {out} {err} (已回滚,记录未保存)",
|
||||
"error",
|
||||
)
|
||||
return redirect(url_for("zone_detail", zone_name=zone_name))
|
||||
|
||||
bind_reload()
|
||||
reload_ok, reload_msg = bind_reload()
|
||||
|
||||
u = current_user()
|
||||
AuditLog.log(u.username, "添加记录", f"域名: {zone_name}, {name} {rtype} {data}")
|
||||
flash(f"记录 {name} {rtype} 添加成功", "success")
|
||||
if reload_ok:
|
||||
flash(f"记录 {name} {rtype} 添加成功", "success")
|
||||
else:
|
||||
# Record IS on disk and zone validated, but BIND refused reload — likely
|
||||
# SERVFAIL or rndc permission error. Surface it loudly so the operator
|
||||
# doesn't think the change took effect.
|
||||
flash(
|
||||
f"记录 {name} {rtype} 已写入,但 BIND 重载失败: {reload_msg}",
|
||||
"error",
|
||||
)
|
||||
return redirect(url_for("zone_detail", zone_name=zone_name))
|
||||
|
||||
|
||||
@@ -961,16 +1030,32 @@ def record_delete(zone_name, idx):
|
||||
abort(404)
|
||||
|
||||
# Delete record from file and bump serial
|
||||
ok, deleted_str, msg = delete_record_from_file(zone["file"], zone_name, idx)
|
||||
ok, deleted_str, msg, original = delete_record_from_file(zone["file"], zone_name, idx)
|
||||
if not ok:
|
||||
flash(f"删除记录失败: {msg}", "error")
|
||||
return redirect(url_for("zone_detail", zone_name=zone_name))
|
||||
|
||||
bind_reload()
|
||||
# Validate zone; roll back on failure (matches record_add behaviour).
|
||||
rc, out, err = run_cmd(f"named-checkzone {zone_name} {zone['file']}")
|
||||
if rc != 0:
|
||||
rollback_zone_file(zone["file"], original)
|
||||
flash(
|
||||
f"Zone 验证失败: {out} {err} (已回滚,记录未删除)",
|
||||
"error",
|
||||
)
|
||||
return redirect(url_for("zone_detail", zone_name=zone_name))
|
||||
|
||||
reload_ok, reload_msg = bind_reload()
|
||||
|
||||
u = current_user()
|
||||
AuditLog.log(u.username, "删除记录", f"域名: {zone_name}, {deleted_str}")
|
||||
flash(f"记录 {deleted_str} 已删除", "success")
|
||||
if reload_ok:
|
||||
flash(f"记录 {deleted_str} 已删除", "success")
|
||||
else:
|
||||
flash(
|
||||
f"记录 {deleted_str} 已从文件删除,但 BIND 重载失败: {reload_msg}",
|
||||
"error",
|
||||
)
|
||||
return redirect(url_for("zone_detail", zone_name=zone_name))
|
||||
|
||||
|
||||
@@ -1293,32 +1378,42 @@ def config_view():
|
||||
def config_options_save():
|
||||
content = request.form.get("content", "")
|
||||
backup = BIND_CONF_OPTIONS + ".bak"
|
||||
|
||||
# Validate BEFORE writing to the live file.
|
||||
# Strategy: write the candidate to BIND_CONF_OPTIONS, validate the WHOLE
|
||||
# named.conf (not just the tmp file — named-checkconf /tmp/foo treats
|
||||
# foo as the master config and refuses 'options-only' directives like
|
||||
# 0.0.0.0 / dnssec-validation / listen-on-v6 at the top level).
|
||||
# If validation fails, restore from backup immediately.
|
||||
run_cmd(f"cp {BIND_CONF_OPTIONS} {backup}")
|
||||
|
||||
# Write to temp and validate
|
||||
# NOTE: BIND_CONF_OPTIONS content must have 'options { ... }' wrapper (it's a complete file)
|
||||
# that gets included from named.conf, so named-checkconf can validate it directly
|
||||
if not re.search(r'^\s*options\s*\{', content, re.MULTILINE):
|
||||
# User may have accidentally removed the options wrapper in text editor
|
||||
content = f"options {{\n{content}\n}};\n"
|
||||
with open("/tmp/named_check.tmp", 'w') as f:
|
||||
f.write(content)
|
||||
rc, out, err = run_cmd("named-checkconf /tmp/named_check.tmp")
|
||||
run_cmd("rm -f /tmp/named_check.tmp")
|
||||
|
||||
if rc != 0:
|
||||
flash(f"配置验证失败: {out} {err}", "error")
|
||||
try:
|
||||
with open(BIND_CONF_OPTIONS, 'w') as f:
|
||||
f.write(content)
|
||||
run_cmd(f"chown root:bind {BIND_CONF_OPTIONS} 2>/dev/null || chown root:named {BIND_CONF_OPTIONS} 2>/dev/null")
|
||||
rc, out, err = run_cmd("named-checkconf")
|
||||
if rc != 0:
|
||||
# Roll back the live file BEFORE the user sees the error flash
|
||||
run_cmd(f"cp {backup} {BIND_CONF_OPTIONS}")
|
||||
run_cmd(f"chown root:bind {BIND_CONF_OPTIONS} 2>/dev/null || chown root:named {BIND_CONF_OPTIONS} 2>/dev/null")
|
||||
flash(f"配置验证失败,已恢复备份: {out} {err}", "error")
|
||||
return redirect(url_for("config_view"))
|
||||
except Exception as e:
|
||||
run_cmd(f"cp {backup} {BIND_CONF_OPTIONS}")
|
||||
flash(f"保存配置失败,已恢复备份: {e}", "error")
|
||||
return redirect(url_for("config_view"))
|
||||
|
||||
with open(BIND_CONF_OPTIONS, 'w') as f:
|
||||
f.write(content)
|
||||
run_cmd(f"chown root:bind {BIND_CONF_OPTIONS} 2>/dev/null || chown root:named {BIND_CONF_OPTIONS} 2>/dev/null")
|
||||
|
||||
bind_reload()
|
||||
reload_ok, reload_msg = bind_reload()
|
||||
|
||||
u = current_user()
|
||||
AuditLog.log(u.username, "修改BIND options配置")
|
||||
flash("options 配置已保存", "success")
|
||||
if reload_ok:
|
||||
flash("options 配置已保存", "success")
|
||||
else:
|
||||
flash(
|
||||
f"options 配置已写入,但 BIND 重载失败: {reload_msg}",
|
||||
"error",
|
||||
)
|
||||
return redirect(url_for("config_view"))
|
||||
|
||||
|
||||
@@ -1424,39 +1519,46 @@ def config_upstream_save():
|
||||
f"recursion {recursion_in};\n",
|
||||
)
|
||||
|
||||
# Validate BEFORE writing to the live file
|
||||
# Ensure content has options block wrapper (required for named-checkconf)
|
||||
if not re.search(r'^\s*options\s*\{', new_content, re.MULTILINE):
|
||||
# No options block found — wrap the entire content
|
||||
new_content = f"options {{\n{new_content}\n}};\n"
|
||||
tmp = "/tmp/named_check.tmp"
|
||||
with open(tmp, 'w') as f:
|
||||
f.write(new_content)
|
||||
rc, out, err = run_cmd(f"named-checkconf {tmp}")
|
||||
run_cmd(f"rm -f {tmp}")
|
||||
if rc != 0:
|
||||
flash(f"配置验证失败: {out} {err}", "error")
|
||||
return redirect(url_for("config_view"))
|
||||
|
||||
# Backup + write
|
||||
# Validate BEFORE writing to the live file.
|
||||
# Strategy: write the candidate to BIND_CONF_OPTIONS, validate the WHOLE
|
||||
# named.conf (named-checkconf /tmp/foo treats foo as the master config
|
||||
# and refuses 'options-only' directives at the top level).
|
||||
# If validation fails, restore from backup immediately.
|
||||
backup = BIND_CONF_OPTIONS + ".bak"
|
||||
run_cmd(f"cp {BIND_CONF_OPTIONS} {backup}")
|
||||
with open(BIND_CONF_OPTIONS, 'w') as f:
|
||||
f.write(new_content)
|
||||
run_cmd(f"chown root:bind {BIND_CONF_OPTIONS} 2>/dev/null || chown root:named {BIND_CONF_OPTIONS} 2>/dev/null")
|
||||
try:
|
||||
with open(BIND_CONF_OPTIONS, 'w') as f:
|
||||
f.write(new_content)
|
||||
run_cmd(f"chown root:bind {BIND_CONF_OPTIONS} 2>/dev/null || chown root:named {BIND_CONF_OPTIONS} 2>/dev/null")
|
||||
rc, out, err = run_cmd("named-checkconf")
|
||||
if rc != 0:
|
||||
run_cmd(f"cp {backup} {BIND_CONF_OPTIONS}")
|
||||
run_cmd(f"chown root:bind {BIND_CONF_OPTIONS} 2>/dev/null || chown root:named {BIND_CONF_OPTIONS} 2>/dev/null")
|
||||
flash(f"配置验证失败,已恢复备份: {out} {err}", "error")
|
||||
return redirect(url_for("config_view"))
|
||||
except Exception as e:
|
||||
run_cmd(f"cp {backup} {BIND_CONF_OPTIONS}")
|
||||
flash(f"保存配置失败,已恢复备份: {e}", "error")
|
||||
return redirect(url_for("config_view"))
|
||||
|
||||
bind_reload()
|
||||
reload_ok, reload_msg = bind_reload()
|
||||
|
||||
u = current_user()
|
||||
AuditLog.log(u.username, "修改上游DNS转发配置",
|
||||
f"forwarders={new_fwd}; allow-recursion={new_rc}; "
|
||||
f"dnssec={dnssec_in}; listen-on={new_listen}; listen-on-v6={listen_on_v6_in}; recursion={recursion_in}")
|
||||
flash(
|
||||
f"已保存:forwarders {len(new_fwd)} 条,allow-recursion {len(new_rc)} 条,"
|
||||
f"dnssec-validation {dnssec_in},listen-on {len(new_listen)} 项,"
|
||||
f"listen-on-v6 {listen_on_v6_in},recursion {recursion_in}",
|
||||
"success",
|
||||
)
|
||||
if reload_ok:
|
||||
flash(
|
||||
f"已保存:forwarders {len(new_fwd)} 条,allow-recursion {len(new_rc)} 条,"
|
||||
f"dnssec-validation {dnssec_in},listen-on {len(new_listen)} 项,"
|
||||
f"listen-on-v6 {listen_on_v6_in},recursion {recursion_in}",
|
||||
"success",
|
||||
)
|
||||
else:
|
||||
flash(
|
||||
f"上游 DNS 配置已写入,但 BIND 重载失败: {reload_msg}",
|
||||
"error",
|
||||
)
|
||||
return redirect(url_for("config_view"))
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user