fix(samba_ad_dc): bind to LAN IP only so the DC stops registering junk DNS

The AD DC's smb.conf set no `interfaces` / `bind interfaces only`, so Samba
bound to — and samba_dnsupdate registered into DNS — every interface on the
host. On myrddin that meant the Docker bridge (172.17.0.1) and k3s/flannel
CNI addresses (10.42.0.0, 10.42.0.1) were published as A records for both the
prole.org apex and `myrddin`, alongside the real 10.0.0.3. Clients then
round-robined onto unroutable addresses, producing the long-standing
"resolves, then doesn't" internal DNS flakiness.

Confine Samba to loopback + the LAN service IP:

  interfaces = lo 10.0.0.3
  bind interfaces only = yes

This scopes both service binding and DNS self-registration to the real
address, so the junk records stop being (re)created on restart.

  - smb.conf.j2: emit the two directives, gated on bind-interfaces-only being
    enabled AND a non-loopback IP being present (empty -> directives omitted,
    never binds loopback-only by accident).
  - defaults: samba_ad_dc_lan_ip ("" by default), samba_ad_dc_bind_interfaces_only
    (true), samba_ad_dc_interfaces (lo + lan_ip), all documented.
  - tasks: assert samba_ad_dc_lan_ip is non-empty before deploying smb.conf
    when bind-interfaces-only is on, so a missing value fails fast instead of
    rendering the DC unreachable.
  - group_vars/ad_dc: set samba_ad_dc_lan_ip=10.0.0.3 (myrddin's LAN address).

Deploying notifies the existing Restart samba-ad-dc handler. Pre-existing junk
records must be deleted once by hand; they will not be re-registered after the
restart. Template rendering verified for both the set and empty-IP cases.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

infrastructure/inventory/group_vars/ad_dc/vars.yml

@@ -2,6 +2,12 @@
 samba_dns_server: "127.0.0.1"
 samba_dns_admin_user: "administrator@PROLE.ORG"
 
+# myrddin's stable LAN service address. Scopes Samba service binding and
+# samba_dnsupdate DNS registration to this IP only, so Docker/k3s interface IPs
+# (172.17.x, 10.42.x) no longer pollute the prole.org zone. If a future child
+# DC joins ad_dc with a different address, move this to host_vars instead.
+samba_ad_dc_lan_ip: "10.0.0.3"
+
 # We'll wire the password with vault next
 samba_dns_admin_pass: "{{ vault_samba_dns_admin_pass }}"
 

infrastructure/roles/samba_ad_dc/defaults/main.yml

@@ -6,6 +6,23 @@ samba_ad_dc_dns_forwarders:
   - 10.0.0.5
   - 10.0.0.4
 
+# --- Interface / DNS-registration scoping ---
+# A Samba AD DC runs samba_dnsupdate on startup (and periodically), which
+# registers an A record for the DC hostname and the realm apex for EVERY IP it
+# is bound to. On a multi-homed host (Docker bridge 172.17.0.1, k3s/flannel
+# 10.42.0.0 / 10.42.0.1, etc.) those junk IPs land in the prole.org zone and
+# get round-robined to clients, producing intermittent "resolves, then doesn't"
+# DNS failures. Binding Samba to loopback + the LAN IP only confines both
+# service binding and DNS registration to the real service address.
+#
+# samba_ad_dc_lan_ip MUST be the DC's stable LAN service address. It is left
+# empty here and set per host (see inventory/group_vars/ad_dc/vars.yml); the
+# role asserts it is non-empty before enabling bind-interfaces-only, so a
+# missing value can never silently bind the DC to loopback alone (unreachable).
+samba_ad_dc_lan_ip: ""
+samba_ad_dc_bind_interfaces_only: true
+samba_ad_dc_interfaces: "{{ ['lo'] + ([samba_ad_dc_lan_ip] if (samba_ad_dc_lan_ip | length > 0) else []) }}"
+
 # Identity
 # When samba_ad_dc_child_id is set (e.g., A000001), the realm/workgroup/netbios
 # will be derived automatically as a child of samba_ad_dc_parent_realm.

infrastructure/roles/samba_ad_dc/tasks/main.yml

@@ -40,6 +40,16 @@
           --username='{{ samba_ad_dc_parent_netbios }}\\Administrator'
   when: not samba_ad_provisioned.stat.exists
 
+- name: Assert LAN IP is set before binding to specific interfaces
+  ansible.builtin.assert:
+    that:
+      - samba_ad_dc_lan_ip | length > 0
+    fail_msg: >-
+      samba_ad_dc_bind_interfaces_only is enabled but samba_ad_dc_lan_ip is empty.
+      Set the DC's LAN service IP (see inventory/group_vars/ad_dc/vars.yml) so
+      Samba does not bind to loopback only and make the DC unreachable.
+  when: samba_ad_dc_bind_interfaces_only | bool
+
 - name: Deploy smb.conf
   ansible.builtin.template:
     src: smb.conf.j2

infrastructure/roles/samba_ad_dc/templates/smb.conf.j2

@@ -1,6 +1,14 @@
 # Global parameters
 [global]
         dns forwarder = {{ samba_ad_dc_dns_forwarders | join(' ') }}
+{% if samba_ad_dc_bind_interfaces_only and (samba_ad_dc_interfaces | reject('equalto', 'lo') | list | length) > 0 %}
+
+        # Confine service binding AND samba_dnsupdate registration to loopback +
+        # the LAN IP. Prevents the DC from registering Docker/CNI interface IPs
+        # (172.17.x, 10.42.x) into the AD DNS zone. See roles/samba_ad_dc/defaults.
+        interfaces = {{ samba_ad_dc_interfaces | join(' ') }}
+        bind interfaces only = yes
+{% endif %}
 
         netbios name = {{ (samba_ad_dc_netbios_name | default(inventory_hostname_short)) | upper }}
         realm = {{ samba_ad_dc_realm }}

infrastructure/roles/samba_dns/tasks/ensure_a.yml

@@ -1,14 +1,29 @@
 ---
-- name: Determine zone and record name
+- 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 }}"
-    samba_dns_record_name: >-
-      {% set fqdn_clean = item.fqdn | regex_replace('\\.?$', '') %}
-      {% if fqdn_clean == ('internal.' ~ prole_domain if item.fqdn.endswith('.internal.' ~ prole_domain) or item.fqdn == 'internal.' ~ prole_domain else prole_domain) %}
-      @
-      {% else %}
-      {{ fqdn_clean | regex_replace('\\.' ~ (('internal.' ~ prole_domain if item.fqdn.endswith('.internal.' ~ prole_domain) or item.fqdn == 'internal.' ~ prole_domain else prole_domain) | regex_escape) ~ '$', '') }}
-      {% endif %}
+
+# 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:
@@ -20,12 +35,7 @@
 
 - name: Parse existing A record values
   ansible.builtin.set_fact:
-    a_existing_values: >-
-      {{
-        (a_query.stdout | default('') |
-         regex_findall('\\bA:\\s+([0-9]{1,3}(?:\\.[0-9]{1,3}){3})\\b') |
-         list)
-      }}
+    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:
@@ -50,3 +60,24 @@
   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 }}"

infrastructure/roles/samba_dns/tasks/ensure_cname.yml

@@ -1,14 +1,24 @@
 ---
-- name: Determine zone and record name
+- 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 }}"
-    samba_dns_record_name: >-
-      {% set fqdn_clean = item.fqdn | regex_replace('\\.?$', '') %}
-      {% if fqdn_clean == ('internal.' ~ prole_domain if item.fqdn.endswith('.internal.' ~ prole_domain) or item.fqdn == 'internal.' ~ prole_domain else prole_domain) %}
-      @
-      {% else %}
-      {{ fqdn_clean | regex_replace('\\.' ~ (('internal.' ~ prole_domain if item.fqdn.endswith('.internal.' ~ prole_domain) or item.fqdn == 'internal.' ~ prole_domain else prole_domain) | regex_escape) ~ '$', '') }}
-      {% endif %}
+
+# See ensure_a.yml for why these are double-quoted scalars and not ">-" blocks:
+# in a block scalar '\\.' reaches the regex as a literal backslash and never
+# matches, leaving the zone suffix un-stripped and the record mis-named.
+- 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 CNAME record name '{{ samba_dns_record_name }}' for {{ item.fqdn }} is empty or contains whitespace."
 
 - name: Query existing CNAME records
   ansible.builtin.command:
@@ -20,12 +30,7 @@
 
 - name: Parse existing CNAME record value
   ansible.builtin.set_fact:
-    cname_existing_value: >-
-      {{
-        (cname_query.stdout | default('') |
-         regex_findall('\\bCNAME:\\s+([a-zA-Z0-9.-]+)\\b') |
-         first | default(''))
-      }}
+    cname_existing_value: "{{ cname_query.stdout | default('') | regex_findall('CNAME:\\s+([A-Za-z0-9.-]+)') | map('regex_replace', '\\.$', '') | first | default('') }}"
 
 - name: Remove stale CNAME record
   ansible.builtin.command:
@@ -33,7 +38,7 @@
       -U Administrator --password={{ samba_dns_admin_pass }}
   when:
     - cname_existing_value | length > 0
-    - cname_existing_value != item.target
+    - cname_existing_value != (item.target | regex_replace('\\.$', ''))
 
 - name: Add missing CNAME record
   ansible.builtin.command:
@@ -45,4 +50,22 @@
     cname_add.rc != 0 and
     ('WERR_DNS_ERROR_RECORD_ALREADY_EXISTS' not in (cname_add.stderr | default(''))) and
     ('Record already exists' not in (cname_add.stderr | default('')))
-  when: cname_existing_value != item.target
+  when: cname_existing_value != (item.target | regex_replace('\\.$', ''))
+
+- name: Verify CNAME record resolves
+  ansible.builtin.command:
+    cmd: samba-tool dns query {{ samba_dns_server }} {{ samba_target_zone }} {{ samba_dns_record_name }} CNAME
+      -U Administrator --password={{ samba_dns_admin_pass }}
+  register: cname_verify
+  changed_when: false
+  failed_when: false
+
+- name: Assert expected CNAME target is present
+  ansible.builtin.assert:
+    that:
+      - cname_verify_value == (item.target | regex_replace('\\.$', ''))
+    fail_msg: >-
+      CNAME {{ samba_dns_record_name }} in zone {{ samba_target_zone }} points to
+      '{{ cname_verify_value }}' but expected '{{ item.target | regex_replace('\\.$', '') }}' after apply.
+  vars:
+    cname_verify_value: "{{ cname_verify.stdout | default('') | regex_findall('CNAME:\\s+([A-Za-z0-9.-]+)') | map('regex_replace', '\\.$', '') | first | default('') }}"

infrastructure/roles/samba_reverse_dns/tasks/ensure_ptr.yml

@@ -6,15 +6,13 @@
   changed_when: false
   failed_when: false
 
+# Double-quoted scalar (not ">-"): in a block scalar '\\s'/'\\.' reach the regex
+# as a literal backslash and never match, so PTR parsing silently returned [] —
+# stale records were never pruned. samba-tool prints PTR targets fully-qualified
+# with a trailing dot, which we strip for comparison against item.fqdn.
 - name: Parse existing PTR records
   ansible.builtin.set_fact:
-    ptr_existing_values: >-
-      {{
-        (ptr_query.stdout | default('') |
-         regex_findall('PTR:\\s+([A-Za-z0-9.-]+)\\.?') |
-         map('regex_replace', '\\.$', '') |
-         list)
-      }}
+    ptr_existing_values: "{{ ptr_query.stdout | default('') | regex_findall('PTR:\\s+([A-Za-z0-9.-]+)') | map('regex_replace', '\\.$', '') | list }}"
 
 - name: Remove stale PTR records
   ansible.builtin.command:
@@ -34,3 +32,20 @@
     ('WERR_DNS_ERROR_RECORD_ALREADY_EXISTS' not in (ptr_add.stderr | default(''))) and
     ('Record already exists' not in (ptr_add.stderr | default('')))
   when: item.fqdn not in ptr_existing_values
+
+- name: Verify PTR record resolves
+  ansible.builtin.command:
+    cmd: samba-tool dns query {{ samba_dns_server }} {{ lan_reverse_zone }} {{ item.last_octet }} PTR -U Administrator --password={{ samba_dns_admin_pass }}
+  register: ptr_verify
+  changed_when: false
+  failed_when: false
+
+- name: Assert expected PTR record is present
+  ansible.builtin.assert:
+    that:
+      - item.fqdn in ptr_verify_values
+    fail_msg: >-
+      PTR {{ item.last_octet }} in zone {{ lan_reverse_zone }} does not contain
+      expected target '{{ item.fqdn }}' after apply (got {{ ptr_verify_values }}).
+  vars:
+    ptr_verify_values: "{{ ptr_verify.stdout | default('') | regex_findall('PTR:\\s+([A-Za-z0-9.-]+)') | map('regex_replace', '\\.$', '') | list }}"

infrastructure/roles/samba_reverse_dns/tasks/main.yml

@@ -13,15 +13,15 @@
   changed_when: false
   tags: [samba, samba_reverse_dns]
 
-- name: Build reverse DNS zone list
-  ansible.builtin.set_fact:
-    samba_reverse_zones: >-
-      {{ ([lan_reverse_zone] + (k3s_reverse_zones | default([]))) | unique }}
-
+# The reverse-zone list is computed inline in the loop below rather than via a
+# separate set_fact. A set_fact must carry the same tags as the tasks that
+# consume it, or tag-filtered runs (e.g. --tags samba_reverse_dns) skip it and
+# leave the variable undefined. Inlining removes that cross-task dependency so
+# the role is correct under any tag selection.
 - name: Create reverse DNS zones if missing
   ansible.builtin.command:
     cmd: samba-tool dns zonecreate {{ samba_dns_server }} {{ reverse_zone }} -U Administrator --password={{ samba_dns_admin_pass }}
-  loop: "{{ samba_reverse_zones }}"
+  loop: "{{ ([lan_reverse_zone] + (k3s_reverse_zones | default([]))) | unique }}"
   loop_control:
     loop_var: reverse_zone
   when: reverse_zone not in samba_zones.stdout