[PATCH backup 1/2] fix #7175: api: time: use timedatectl instead of /etc/timezone

[PATCH backup 1/2] fix #7175: api: time: use timedatectl instead of /etc/timezone

[Maximiliano Sandoval]

The Proxmox Backup Server 4.2 iso does not create /etc/timezone, so we
use timedatectl as recommended at [1].

As per timedatectl(1) set-timezone will alter the /etc/localtime
symlink, and we return early on set_timezone() if the command succeeds.

[1] https://salsa.debian.org/glibc-team/tzdata/-/blob/trixie/debian/README.Debian?ref_type=heads#L54

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---

Tested:
    - proxmox-backup-debug api get /nodes/{node}/time
    - proxmox-backup-debug api set /nodes/{node}/time --timezone $tz
    - Checked that /etc/timezone is not created
    - Captured dbus output via busctl capture > output.pcapng and checked
      that the org.freedesktop.timedate1 interface (used by timedatectl) is
      being used for {g,s}etting the timezone.
    - Verified that the symlink at /etc/localtime points to the right place.
    
Note that at the moment both getting and setting the timezone work as
expected without this patch, in practice the only difference is that
/etc/timezone will not be created anymore after setting the timezone.

 src/api2/node/time.rs | 74 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 63 insertions(+), 11 deletions(-)

diff --git a/src/api2/node/time.rs b/src/api2/node/time.rs
index 6baca378e..9ab4e84d8 100644
--- a/src/api2/node/time.rs
+++ b/src/api2/node/time.rs
@@ -1,16 +1,16 @@
+use std::process::Command;
+
 use anyhow::{Error, bail, format_err};
 use serde_json::{Value, json};
 
 use proxmox_router::{Permission, Router};
 use proxmox_schema::api;
-use proxmox_sys::fs::{CreateOptions, file_read_firstline, replace_file};
 
 use pbs_api_types::{NODE_SCHEMA, PRIV_SYS_MODIFY, TIME_ZONE_SCHEMA};
 
 fn read_etc_localtime() -> Result<String, Error> {
-    // use /etc/timezone
-    if let Ok(line) = file_read_firstline("/etc/timezone") {
-        return Ok(line.trim().to_owned());
+    if let Ok(timezone) = timedatectl_get_timezone() {
+        return Ok(timezone);
     }
 
     // otherwise guess from the /etc/localtime symlink
@@ -24,6 +24,61 @@ fn read_etc_localtime() -> Result<String, Error> {
     }
 }
 
+fn timedatectl_get_timezone() -> Result<String, Error> {
+    let output = Command::new("timedatectl")
+        .args(["show", "--property=Timezone", "--value"])
+        .output()
+        .map_err(|err| format_err!("failed to execute timedatectl show - {err}"))?;
+
+    if !output.status.success() {
+        if let Some(code) = output.status.code() {
+            let msg = String::from_utf8(output.stderr)
+                .map(|s| {
+                    if s.is_empty() {
+                        String::from("no error message")
+                    } else {
+                        s
+                    }
+                })
+                .unwrap_or_else(|_| String::from("non utf8 error message (suppressed)"));
+            bail!("timedatectl show failed with status code {code} - {msg}",);
+        } else {
+            bail!("timedatectl terminated by signal",);
+        }
+    }
+
+    let timezone = String::from_utf8(output.stdout)
+        .map_err(|err| format_err!("non utf8 timezone from timedatectl - {err}"))?;
+
+    Ok(timezone)
+}
+
+fn timedatectl_set_timezone(timezone: &str) -> Result<(), Error> {
+    let output = Command::new("timedatectl")
+        .args(["set-timezone", timezone])
+        .output()
+        .map_err(|err| format_err!("failed to execute timedatectl set-timezone - {err}"))?;
+
+    if !output.status.success() {
+        if let Some(code) = output.status.code() {
+            let msg = String::from_utf8(output.stderr)
+                .map(|s| {
+                    if s.is_empty() {
+                        String::from("no error message")
+                    } else {
+                        s
+                    }
+                })
+                .unwrap_or_else(|_| String::from("non utf8 error message (suppressed)"));
+            bail!("timedatectl set-timezone failed with status code {code} - {msg}",);
+        } else {
+            bail!("timedatectl terminated by signal",);
+        }
+    }
+
+    Ok(())
+}
+
 #[api(
     input: {
         properties: {
@@ -88,19 +143,16 @@ fn get_time(_param: Value) -> Result<Value, Error> {
 )]
 /// Set time zone
 fn set_timezone(timezone: String, _param: Value) -> Result<Value, Error> {
+    if timedatectl_set_timezone(&timezone).is_ok() {
+        return Ok(Value::Null);
+    }
+
     let path = std::path::PathBuf::from(format!("/usr/share/zoneinfo/{timezone}"));
 
     if !path.exists() {
         bail!("No such timezone.");
     }
 
-    replace_file(
-        "/etc/timezone",
-        timezone.as_bytes(),
-        CreateOptions::new(),
-        true,
-    )?;
-
     let _ = std::fs::remove_file("/etc/localtime");
 
     use std::os::unix::fs::symlink;
-- 
2.47.3

[PATCH backup 2/2] api: time: rename read_etc_localtime

[Maximiliano Sandoval]

The function tries to get the timezone from timedatectl and fallbacks to
/etc/localtime, so we rename it to get_timezone.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---

Notes:
    Whether timedatectl uses /etc/localtime is an implementation detail, but
    please feel free to drop this commit if it does not add much.

 src/api2/node/time.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/api2/node/time.rs b/src/api2/node/time.rs
index 9ab4e84d8..55de0d8ff 100644
--- a/src/api2/node/time.rs
+++ b/src/api2/node/time.rs
@@ -8,7 +8,7 @@ use proxmox_schema::api;
 
 use pbs_api_types::{NODE_SCHEMA, PRIV_SYS_MODIFY, TIME_ZONE_SCHEMA};
 
-fn read_etc_localtime() -> Result<String, Error> {
+fn get_timezone() -> Result<String, Error> {
     if let Ok(timezone) = timedatectl_get_timezone() {
         return Ok(timezone);
     }
@@ -118,7 +118,7 @@ fn get_time(_param: Value) -> Result<Value, Error> {
     let localtime = time + offset;
 
     Ok(json!({
-        "timezone": read_etc_localtime()?,
+        "timezone": get_timezone()?,
         "time": time,
         "localtime": localtime,
     }))
-- 
2.47.3

Re: [PATCH backup 1/2] fix #7175: api: time: use timedatectl instead of /etc/timezone

[Maximiliano Sandoval]

Maximiliano Sandoval <m.sandoval@proxmox.com> writes:

> The Proxmox Backup Server 4.2 iso does not create /etc/timezone, so we
> use timedatectl as recommended at [1].
>
> As per timedatectl(1) set-timezone will alter the /etc/localtime
> symlink, and we return early on set_timezone() if the command succeeds.
>
> [1] https://salsa.debian.org/glibc-team/tzdata/-/blob/trixie/debian/README.Debian?ref_type=heads#L54
>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>
> [..]

I forgot to mention that this is a continuation of [1].

[1] https://lore.proxmox.com/all/20260125210151.1151-1-stefan@mayr-stefan.de/T/#u

-- 
Maximiliano