dns: T6422: allow multiple redundant NS records

[haimgel]

Change Summary

NS is unlike CNAME or PTR, multiple NS records are perfectly valid and is a common use case: multiple redundant DNS servers is a common configuration and should be supported.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Code style update (formatting, renaming)
• Refactoring (no functional changes)
• Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
• Other (please describe):

Component(s) name

dns

Proposed changes

Make NS record configuration behave more like A record, where multiple records with the same target are allowed.

How to test

set service dns forwarding authoritative-domain example.com records ns test target 'ns1.example.com'
set service dns forwarding authoritative-domain example.com records ns test target 'ns2.example.com'
commit

dig ns test.example.com

Response:
;; ANSWER SECTION:
test.example.com.	254	IN	NS	ns2.example.com.
test.example.com.	254	IN	NS	ns1.example.com.

Smoketest result

$ /usr/libexec/vyos/tests/smoke/cli/test_service_dns_forwarding.py
test_basic_forwarding (__main__.TestServicePowerDNS.test_basic_forwarding) ...
DNS forwarding requires a listen-address


DNS forwarding requires a listen-address

ok
test_dns64 (__main__.TestServicePowerDNS.test_dns64) ...
DNS 6to4 prefix must be of length /96

ok
test_dnssec (__main__.TestServicePowerDNS.test_dnssec) ... ok
test_domain_forwarding (__main__.TestServicePowerDNS.test_domain_forwarding) ... ok
test_ecs_add_for (__main__.TestServicePowerDNS.test_ecs_add_for) ... ok
test_ecs_ipv4_bits (__main__.TestServicePowerDNS.test_ecs_ipv4_bits) ... ok
test_edns_subnet_allow_list (__main__.TestServicePowerDNS.test_edns_subnet_allow_list) ... ok
test_exclude_throttle_adress (__main__.TestServicePowerDNS.test_exclude_throttle_adress) ... ok
test_external_nameserver (__main__.TestServicePowerDNS.test_external_nameserver) ... ok
test_listening_port (__main__.TestServicePowerDNS.test_listening_port) ... ok
test_multiple_ns_records (__main__.TestServicePowerDNS.test_multiple_ns_records) ... ok
test_no_rfc1918_forwarding (__main__.TestServicePowerDNS.test_no_rfc1918_forwarding) ... ok
test_serve_stale_extension (__main__.TestServicePowerDNS.test_serve_stale_extension) ... ok

----------------------------------------------------------------------
Ran 13 tests in 230.559s

OK

Checklist:

• I have read the CONTRIBUTING document
• I have linked this PR to one or more Phabricator Task(s)
• I have run the components SMOKETESTS if applicable
• My commit headlines contain a valid Task id
• My change requires a change to the documentation
• I have updated the documentation accordingly

[dmbaturin]

Please add a smoke test, too!

[haimgel]

@dmbaturin, thanks! I added a smoke test and fixed the typo. Note that smoke coverage for authoritative DNS setup is not great to begin with, I focused here on what this change does only.

[c-po]

Please update the PR description with a more recent output of your smoketest, as the provided one lacks your newly added testcase.

other then that, LGTM

[haimgel]

Please update the PR description with a more recent output of your smoketest, as the provided one lacks your newly added testcase.

other then that, LGTM

Thank you. Done.

[c-po]

@Mergifyio backport sagitta

[mergify]

backport sagitta

✅ Backports have been created

• #3568 dns: T6422: allow multiple redundant NS records (backport #3557) has been created for branch sagitta

Changes implemented