[pve-devel] [PATCH installer 0/3] run env, auto: fix dhcp hostname/domain retrieval

[pve-devel] [PATCH installer 0/3] run env, auto: fix dhcp hostname/domain retrieval

[Christoph Heiss]

First two patches fix two small, separate issues w.r.t. the automatic
retrieval of hostname and search domain from the DHCP lease.

First one is a correctness fix for RFC 2132, which specifies that the
hostname in the DHCP lease _can_ also contain a fully-qualified name,
i.e. <hostname>.<domain>. That can easily be stripped.

Second was that in the auto-installer answer file `global.fqdn.domain`
can be set to an empty string, which further down the line causes
parsing failure and the default `<productname>.example.invalid` being
set as system FQDN.

Both were reported by a user on the community forum [0].

Third patch just runs perltidy on the codebase again, fixing up some
recent changes.

[0] https://forum.proxmox.com/threads/auto-install-fetching-fqdn-through-dhcp-does-not-set-search-domain-correctly.168369/

Diffstat
========

Christoph Heiss (3):
  run env: fix dhcp-set hostname containing local domain
  auto: answer: deserialize empty domain name as `None`
  install: run `make tidy`

 Proxmox/Install.pm                            |  6 ++++--
 Proxmox/Install/RunEnv.pm                     |  8 +++++++-
 Proxmox/Sys/Net.pm                            | 12 ++++++++----
 proxmox-auto-installer/src/answer.rs          | 13 +++++++++++++
 proxmox-auto-installer/tests/parse-answer.rs  |  1 +
 ...n_from_dhcp_empty_dhcp_domain_setting.json | 19 +++++++++++++++++++
 ...n_from_dhcp_empty_dhcp_domain_setting.toml | 17 +++++++++++++++++
 7 files changed, 69 insertions(+), 7 deletions(-)
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_empty_dhcp_domain_setting.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_empty_dhcp_domain_setting.toml

-- 
2.49.0


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH installer 1/3] run env: fix dhcp-set hostname containing local domain

[Christoph Heiss]

According to RFC 2132 section 3.14 [0], the hostname _can_ also include the
local domain name. Strip the suffix it before storing it into the
runtime environment.

Reported on the community forum [1].

[0] https://www.rfc-editor.org/rfc/rfc2132#section-3.14
[1] https://forum.proxmox.com/threads/auto-install-fetching-fqdn-through-dhcp-does-not-set-search-domain-correctly.168369/

Fixes: bda1cdf ("run env: retrieve and store hostname from DHCP lease if available")
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 Proxmox/Install/RunEnv.pm |  7 ++++++-
 Proxmox/Sys/Net.pm        | 12 ++++++++----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index cdd7992..ce89346 100644
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -292,7 +292,12 @@ sub query_installation_environment : prototype() {
     };
 
     # avoid serializing out null or an empty string, that can trip up the UIs
-    if (my $fqdn = Proxmox::Sys::Net::get_dhcp_fqdn()) {
+    if (my $fqdn = Proxmox::Sys::Net::get_dhcp_hostname()) {
+        if (defined($output->{network}->{dns}->{domain})) {
+            # strip domain name suffix, if possible
+            $fqdn =~ s/\.$output->{network}->{dns}->{domain}//;
+        }
+
         $output->{network}->{hostname} = $fqdn;
     }
 
diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
index 38aab0e..6fe99ec 100644
--- a/Proxmox/Sys/Net.pm
+++ b/Proxmox/Sys/Net.pm
@@ -204,12 +204,16 @@ sub udevadm_netdev_details {
     return $result;
 }
 
-# Tries to detect the FQDN hostname for this system via DHCP, if available.
+# Tries to detect the hostname for this system given via DHCP, if available.
+# The hostname _might_ also include the local domain name, depending on the
+# concrete DHCP server implementation.
+#
+# DHCP servers can set option 12 to inform the client about it's hostname [0].
+# dhclient dumps all options set by the DHCP server it in lease file, so just
+# read it from there.
 #
-# DHCP server can set option 12 to inform the client about it's hostname [0]. dhclient dumps all
-# options set by the DHCP server it in lease file, so just read it from there.
 # [0] RFC 2132, section 3.14
-sub get_dhcp_fqdn : prototype() {
+sub get_dhcp_hostname : prototype() {
     my $leasefile = '/var/lib/dhcp/dhclient.leases';
     return if !-f $leasefile;
 
-- 
2.49.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH installer 2/3] auto: answer: deserialize empty domain name as `None`

[Christoph Heiss]

This handles the case where users set `global.fqdn.domain = ""`, which
would result incorrect parsing and finally resulting in the FQDN of the
target machine being set to `<productname>.example.invalid`.

Reported on the community forum [0].

[0] https://forum.proxmox.com/threads/auto-install-fetching-fqdn-through-dhcp-does-not-set-search-domain-correctly.168369/

Fixes: a37e044 ("fix #5811: auto: add option to retrieve FQDN from DHCP configuration")
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-auto-installer/src/answer.rs          | 13 +++++++++++++
 proxmox-auto-installer/tests/parse-answer.rs  |  1 +
 ...n_from_dhcp_empty_dhcp_domain_setting.json | 19 +++++++++++++++++++
 ...n_from_dhcp_empty_dhcp_domain_setting.toml | 17 +++++++++++++++++
 4 files changed, 50 insertions(+)
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_empty_dhcp_domain_setting.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_empty_dhcp_domain_setting.toml

diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
index b461976..88f4c87 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -90,6 +90,7 @@ pub struct FqdnExtendedConfig {
     #[serde(default)]
     pub source: FqdnSourceMode,
     /// Domain to use if none is received via DHCP.
+    #[serde(default, deserialize_with = "deserialize_non_empty_string_maybe")]
     pub domain: Option<String>,
 }
 
@@ -444,3 +445,15 @@ pub enum KeyboardLayout {
 }
 
 serde_plain::derive_display_from_serialize!(KeyboardLayout);
+
+fn deserialize_non_empty_string_maybe<'de, D>(deserializer: D) -> Result<Option<String>, D::Error>
+where
+    D: serde::Deserializer<'de>,
+{
+    let val: Option<String> = Deserialize::deserialize(deserializer)?;
+
+    match val {
+        Some(s) if !s.is_empty() => Ok(Some(s)),
+        _ => Ok(None),
+    }
+}
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 88f30aa..a354f21 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -124,6 +124,7 @@ mod tests {
             disk_match_any,
             first_boot,
             fqdn_from_dhcp,
+            fqdn_from_dhcp_empty_dhcp_domain_setting,
             fqdn_from_dhcp_no_dhcp_domain_with_default_domain,
             full_fqdn_from_dhcp_with_default_domain,
             hashed_root_password,
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_empty_dhcp_domain_setting.json b/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_empty_dhcp_domain_setting.json
new file mode 100644
index 0000000..5ec6656
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_empty_dhcp_domain_setting.json
@@ -0,0 +1,19 @@
+{
+  "autoreboot": 1,
+  "cidr": "192.168.1.114/24",
+  "country": "at",
+  "dns": "192.168.1.254",
+  "domain": "test.local",
+  "filesys": "ext4",
+  "gateway": "192.168.1.1",
+  "hdsize": 223.57088470458984,
+  "existing_storage_auto_rename": 1,
+  "hostname": "pveauto",
+  "keymap": "de",
+  "mailto": "mail@no.invalid",
+  "mngmt_nic": "eno1",
+  "root_password": { "plain": "12345678" },
+  "target_hd": "/dev/sda",
+  "timezone": "Europe/Vienna",
+  "first_boot": { "enabled": 0 }
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_empty_dhcp_domain_setting.toml b/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_empty_dhcp_domain_setting.toml
new file mode 100644
index 0000000..a19e342
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/fqdn_from_dhcp_empty_dhcp_domain_setting.toml
@@ -0,0 +1,17 @@
+[global]
+keyboard = "de"
+country = "at"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[global.fqdn]
+source = "from-dhcp"
+domain = ""
+
+[network]
+source = "from-dhcp"
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
-- 
2.49.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH installer 3/3] install: run `make tidy`

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 Proxmox/Install.pm        | 6 ++++--
 Proxmox/Install/RunEnv.pm | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 09ee724..c15251f 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -1271,8 +1271,10 @@ _EOD
                     && !Proxmox::Install::Config::get_first_boot_opt('enabled'));
 
             # support installing a matching CPU microcode package by default
-            next if ($deb =~ /^amd64-microcode_/ && $run_env->{cpu_vendor_id} ne 'AuthenticAMD');
-            next if ($deb =~ /^intel-microcode_/ && $run_env->{cpu_vendor_id} ne 'GenuineIntel');
+            next
+                if ($deb =~ /^amd64-microcode_/ && $run_env->{cpu_vendor_id} ne 'AuthenticAMD');
+            next
+                if ($deb =~ /^intel-microcode_/ && $run_env->{cpu_vendor_id} ne 'GenuineIntel');
 
             update_progress($count / $pkg_count, 0.5, 0.75, "extracting $deb");
 
diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index ce89346..16c02af 100644
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -39,6 +39,7 @@ sub query_total_memory : prototype() {
 }
 
 my $_cached_cpu_info = undef;
+
 sub query_cpu_info : prototype() {
     return $_cached_cpu_info if defined($_cached_cpu_info);
 
-- 
2.49.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [PATCH installer 0/3] run env, auto: fix dhcp hostname/domain retrieval

[Thomas Lamprecht]

On Tue, 15 Jul 2025 15:55:38 +0200, Christoph Heiss wrote:
> First two patches fix two small, separate issues w.r.t. the automatic
> retrieval of hostname and search domain from the DHCP lease.
> 
> First one is a correctness fix for RFC 2132, which specifies that the
> hostname in the DHCP lease _can_ also contain a fully-qualified name,
> i.e. <hostname>.<domain>. That can easily be stripped.
> 
> [...]

Applied, thanks!

[1/3] run env: fix dhcp-set hostname containing local domain
      commit: a9c0a445ab7cc7848e5e6271365cdd69abd9eb03
[2/3] auto: answer: deserialize empty domain name as `None`
      commit: 3d81300ca62246989bc18991c85a67d1234b1a97
[3/3] install: run `make tidy`
      commit: 84b952b14aa18c00172eac28162674f897fc0511


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel