prole/infrastructure/roles/samba_dns/tasks/ensure_a.yml
chrisfu c0c43bcf6a fix(samba_dns): strip zone suffix correctly so internal A/CNAME/PTR records are created
Forward A/CNAME and reverse PTR records reported changed=0 / ok but never
resolved by short name on the Samba AD DC (myrddin.prole.org). Root cause was
regex escaping inside YAML folded block scalars (">-"): unlike double-quoted
scalars, block scalars do NOT process backslash escapes, so a pattern written
as '\\.' reached Jinja as a literal backslash + any-char and never matched.

Consequences:
  - regex_replace() never stripped the zone suffix, so records were created
    under their full FQDN (e.g. a record literally named "sg2428lp.prole.org"
    inside zone "prole.org"). `samba-tool dns query ... prole.org sg2428lp A`
    then returns WERR_DNS_ERROR_NAME_DOES_NOT_EXIST.
  - regex_findall() of existing values always returned [], so stale records
    were never pruned and idempotency only survived via RECORD_ALREADY_EXISTS.

The leading/trailing whitespace from the folded scalar was a red herring: the
command module's shlex tokenization collapses it, so a single-token name
survived intact. The escaping was the actual defect.

Fix:
  - Convert the templated set_fact expressions from ">-" block scalars to
    double-quoted single-line scalars (one consistent escaping convention,
    matching the already-working samba_target_zone line; also removes the
    stray whitespace).
  - Add an assertion that the computed record name is a clean, non-empty,
    whitespace-free token.
  - Add a post-apply verification re-query + assert that the expected
    value(s) are actually present, turning a silent no-op into a hard failure.

Applies to roles/samba_dns (ensure_a.yml, ensure_cname.yml) and
roles/samba_reverse_dns (ensure_ptr.yml).

Verified end-to-end through ansible-playbook against a stateful fake
samba-tool: correct short names (sg2428lp, git, @, registry), stale-value
removal, internal.prole.org zone routing, idempotent re-runs (changed=0),
and the verify-assert failing loudly when an add silently does not persist.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-10 00:32:00 -07:00

84 lines
3.9 KiB
YAML

---
- name: Determine target zone
ansible.builtin.set_fact:
samba_target_zone: "{{ 'internal.' ~ prole_domain if item.fqdn.endswith('.internal.' ~ prole_domain) or item.fqdn == 'internal.' ~ prole_domain else prole_domain }}"
# NOTE: these expressions are deliberately double-quoted single-line scalars,
# NOT folded block scalars (">-"). In a block scalar YAML does not process
# backslash escapes, so a regex written as '\\.' reaches Jinja as the two
# characters "\\." (literal-backslash + any-char) and silently never matches —
# which previously caused the zone suffix NOT to be stripped, so records were
# created under their full FQDN (e.g. "sg2428lp.prole.org" inside zone
# "prole.org") and were unqueryable by short name. In a double-quoted scalar
# YAML collapses '\\.' -> '\.', which is the correct regex. Keep them this way.
- name: Determine record name (relative label; '@' for the zone apex)
ansible.builtin.set_fact:
samba_dns_record_name: "{{ '@' if fqdn_clean == samba_target_zone else (fqdn_clean | regex_replace('\\.' ~ (samba_target_zone | regex_escape) ~ '$', '')) }}"
vars:
fqdn_clean: "{{ item.fqdn | regex_replace('\\.$', '') }}"
- name: Assert record name is a clean single token
ansible.builtin.assert:
that:
- samba_dns_record_name | length > 0
- samba_dns_record_name == (samba_dns_record_name | trim)
- "' ' not in samba_dns_record_name"
fail_msg: "Computed A record name '{{ samba_dns_record_name }}' for {{ item.fqdn }} is empty or contains whitespace."
- name: Query existing A records
ansible.builtin.command:
cmd: samba-tool dns query {{ samba_dns_server }} {{ samba_target_zone }} {{ samba_dns_record_name }} A
-U Administrator --password={{ samba_dns_admin_pass }}
register: a_query
changed_when: false
failed_when: false
- name: Parse existing A record values
ansible.builtin.set_fact:
a_existing_values: "{{ a_query.stdout | default('') | regex_findall('A:\\s+([0-9]{1,3}(?:\\.[0-9]{1,3}){3})') | list }}"
- name: Remove stale A records
ansible.builtin.command:
cmd: samba-tool dns delete {{ samba_dns_server }} {{ samba_target_zone }} {{ samba_dns_record_name }} A {{ a_value }}
-U Administrator --password={{ samba_dns_admin_pass }}
loop: "{{ a_existing_values | difference(item.ipv4s) }}"
loop_control:
loop_var: a_value
when: (a_existing_values | difference(item.ipv4s) | length) > 0
- name: Add missing A records
ansible.builtin.command:
cmd: samba-tool dns add {{ samba_dns_server }} {{ samba_target_zone }} {{ samba_dns_record_name }} A {{ a_target_ip }}
-U Administrator --password={{ samba_dns_admin_pass }}
register: a_add
changed_when: a_add.rc == 0
failed_when: >
a_add.rc != 0 and
('WERR_DNS_ERROR_RECORD_ALREADY_EXISTS' not in (a_add.stderr | default(''))) and
('Record already exists' not in (a_add.stderr | default('')))
loop: "{{ item.ipv4s | difference(a_existing_values) }}"
loop_control:
loop_var: a_target_ip
when: (item.ipv4s | difference(a_existing_values) | length) > 0
# Verification: re-query and assert every expected IP is now present. This turns
# a silently-broken name/regex into a hard failure instead of a false "ok".
- name: Verify A records resolve
ansible.builtin.command:
cmd: samba-tool dns query {{ samba_dns_server }} {{ samba_target_zone }} {{ samba_dns_record_name }} A
-U Administrator --password={{ samba_dns_admin_pass }}
register: a_verify
changed_when: false
failed_when: false
- name: Assert expected A record values are present
ansible.builtin.assert:
that:
- (item.ipv4s | difference(a_verify_values) | length) == 0
fail_msg: >-
A record {{ samba_dns_record_name }} in zone {{ samba_target_zone }} is missing
expected IP(s) {{ item.ipv4s | difference(a_verify_values) }} after apply
(got {{ a_verify_values }}).
vars:
a_verify_values: "{{ a_verify.stdout | default('') | regex_findall('A:\\s+([0-9]{1,3}(?:\\.[0-9]{1,3}){3})') | list }}"