Re: [PATCH common] fix #6357: inotify: resolv.conf: parse all search domains

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 07.11.26 um 23:30 schrieb Lucas Barros:
> The 'search' line of /etc/resolv.conf can list multiple domains, but
> the parser only kept the first one. Writing the DNS settings of a node
> with multiple search domains worked, but reading them back, for example
> in the node's DNS panel in the web UI, only showed the first domain, so
> saving the dialog again silently dropped the others.
> 
> Keep the whole list, with the domains separated by a single space. This
> is the format the DNS API already accepts when writing, and the in-tree
> users of the value that fall back to the host's search domains for
> guests, cloud-init in qemu-server and the container setup code in
> pve-container, already split it on whitespace.

Thanks for your patch, this can be fine, but needs some special care to
ensure we do not have any use site where this causes problems, see
inline below.

> 
> Add a small test for the parser.
> 
> Signed-off-by: Lucas Barros <lucasbarros.kam@gmail.com>
> ---
> 
> The users of the parsed value that I found are the DNS API of the node
> in pve-manager, get_dns_conf() in qemu-server's cloud-init code and
> get_dns_settings() / update_etc_hosts() in pve-container's setup code.
> The last two already treat it as a whitespace separated list. Note that
> this means guests without their own 'searchdomain' now get all search
> domains of the host instead of only the first one, both in their
> resolv.conf and, for containers, as additional names in /etc/hosts.
> That is what the host itself uses, so I think it is the expected
> behavior, but please tell me if you prefer to keep that path as is.
> 
> Ran the new test/resolv_conf_test.pl, which fails on the parser before
> this change (3 of 7) and passes with it. I ran it on perl 5.34 with
> stubs for the Linux-only modules (Linux::Inotify2, syscall.ph), not on
> a Debian system, and did not run the rest of 'make check'. proxmox-
> perltidy was not available, the code was formatted by hand.
> 
> This patch was prepared with the help of an LLM based coding assistant; I
> reviewed the change and ran the tests mentioned above myself.
> 
>  src/PVE/INotify.pm       |  4 +--
>  test/Makefile            |  1 +
>  test/resolv_conf_test.pl | 65 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 68 insertions(+), 2 deletions(-)
>  create mode 100755 test/resolv_conf_test.pl
> 
> diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
> index 53b4e21..4a0355e 100644
> --- a/src/PVE/INotify.pm
> +++ b/src/PVE/INotify.pm
> @@ -609,8 +609,8 @@ sub read_etc_resolv_conf {
>      my $nscount = 0;
>      while (my $line = <$fh>) {
>          chomp $line;
> -        if ($line =~ m/^(search|domain)\s+(\S+)\s*/) {
> -            $res->{search} = $2;
> +        if ($line =~ m/^(search|domain)\s+(\S.*)$/) {
> +            $res->{search} = join(' ', split(/\s+/, $2));

The node certificate generator in src/PVE/Cluster/Setup.pm builds the
certificate FQDN by appending the whole search value to the node name, for
both the subject CN and a DNS subject alternative name. With a multi-domain
search list this now yields a CN and SAN like `node.a.example b.example` -
so that needs to cope with this. Maybe there are other sites, checking the
behavior in the rust side (PBS, PDM) might be good to, even just to ensure
consistency.

>          } elsif ($line =~ m/^\s*nameserver\s+($PVE::Tools::IPRE)\s*/) {
>              $nscount++;
>              if ($nscount <= 3) {
> diff --git a/test/Makefile b/test/Makefile
> index 9caba10..416bc0c 100644
> --- a/test/Makefile
> +++ b/test/Makefile
> @@ -19,6 +19,7 @@ TESTS = lock_file.test			\
>  	ticket_test.test \
>  	check_pem_test.test \
>  	split_pem_test.test \
> +	resolv_conf_test.test \
>  
>  all:
>