libpod: fix timezone handling

The current way of bind mounting the host timezone file has problems.
Because /etc/localtime in the image may exist and is a symlink under
/usr/share/zoneinfo it will overwrite the targetfile. That confuses
timezone parses especially java where this approach does not work at
all. So we end up with an link which does not reflect the actual truth.

The better way is to just change the symlink in the image like it is
done on the host. However because not all images ship tzdata we cannot
rely on that either. So now we do both, when tzdata is installed then
use the symlink and if not we keep the current way of copying the host
timezone file in the container to /etc/localtime.

Also note that we need to rebuild the systemd image to include tzdata in
order to test this as our images do not contain the tzdata by default.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2149876

Signed-off-by: Paul Holzinger <[email protected]>

libpod/container_internal.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -1669,6 +1670,48 @@ func (c *Container) mountStorage() (_ string, deferredErr error) {
 		}
 	}
 
+	tz := c.Timezone()
+	if tz != "" {
+		timezonePath := filepath.Join("/usr/share/zoneinfo", tz)
+		if tz == "local" {
+			timezonePath, err = filepath.EvalSymlinks("/etc/localtime")
+			if err != nil {
+				return "", fmt.Errorf("finding local timezone for container %s: %w", c.ID(), err)
+			}
+		}
+		// make sure to remove any existing localtime file in the container to not create invalid links
+		err = unix.Unlinkat(etcInTheContainerFd, "localtime", 0)
+		if err != nil && !errors.Is(err, fs.ErrNotExist) {
+			return "", fmt.Errorf("removing /etc/localtime: %w", err)
+		}
+
+		hostPath, err := securejoin.SecureJoin(mountPoint, timezonePath)
+		if err != nil {
+			return "", fmt.Errorf("resolve zoneinfo path in the container: %w", err)
+		}
+
+		_, err = os.Stat(hostPath)
+		if err != nil {
+			// file does not exists which means tzdata is not installed in the container, just create /etc/locatime which a copy from the host
+			logrus.Debugf("Timezone %s does not exist in the container, create our own copy from the host", timezonePath)
+			localtimePath, err := c.copyTimezoneFile(timezonePath)
+			if err != nil {
+				return "", fmt.Errorf("setting timezone for container %s: %w", c.ID(), err)
+			}
+			if c.state.BindMounts == nil {
+				c.state.BindMounts = make(map[string]string)
+			}
+			c.state.BindMounts["/etc/localtime"] = localtimePath
+		} else {
+			// file exists lets just symlink according to localtime(5)
+			logrus.Debugf("Create locatime symlink for %s", timezonePath)
+			err = unix.Symlinkat(".."+timezonePath, etcInTheContainerFd, "localtime")
+			if err != nil {
+				return "", fmt.Errorf("creating /etc/localtime symlink: %w", err)
+			}
+		}
+	}
+
 	// Request a mount of all named volumes
 	for _, v := range c.config.NamedVolumes {
 		vol, err := c.mountNamedVolume(v, mountPoint)

libpod/container_internal_common.go

@@ -1904,38 +1904,6 @@ func (c *Container) makeBindMounts() error {
 		}
 	}
 
-	// Make /etc/localtime
-	ctrTimezone := c.Timezone()
-	if ctrTimezone != "" {
-		// validate the format of the timezone specified if it's not "local"
-		if ctrTimezone != "local" {
-			_, err = time.LoadLocation(ctrTimezone)
-			if err != nil {
-				return fmt.Errorf("finding timezone for container %s: %w", c.ID(), err)
-			}
-		}
-		if _, ok := c.state.BindMounts["/etc/localtime"]; !ok {
-			var zonePath string
-			if ctrTimezone == "local" {
-				zonePath, err = filepath.EvalSymlinks("/etc/localtime")
-				if err != nil {
-					return fmt.Errorf("finding local timezone for container %s: %w", c.ID(), err)
-				}
-			} else {
-				zone := filepath.Join("/usr/share/zoneinfo", ctrTimezone)
-				zonePath, err = filepath.EvalSymlinks(zone)
-				if err != nil {
-					return fmt.Errorf("setting timezone for container %s: %w", c.ID(), err)
-				}
-			}
-			localtimePath, err := c.copyTimezoneFile(zonePath)
-			if err != nil {
-				return fmt.Errorf("setting timezone for container %s: %w", c.ID(), err)
-			}
-			c.state.BindMounts["/etc/localtime"] = localtimePath
-		}
-	}
-
 	_, hasRunContainerenv := c.state.BindMounts["/run/.containerenv"]
 	if !hasRunContainerenv {
 	Loop:

libpod/options.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"strings"
 	"syscall"
+	"time"
 
 	"github.com/containers/buildah/pkg/parse"
 	nettypes "github.com/containers/common/libnetwork/types"
@@ -1814,15 +1815,10 @@ func WithTimezone(path string) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 		if path != "local" {
-			zone := filepath.Join("/usr/share/zoneinfo", path)
-
-			file, err := os.Stat(zone)
+			// validate the format of the timezone specified if it's not "local"
+			_, err := time.LoadLocation(path)
 			if err != nil {
-				return err
-			}
-			// We don't want to mount a timezone directory
-			if file.IsDir() {
-				return errors.New("invalid timezone: is a directory")
+				return fmt.Errorf("finding timezone: %w", err)
 			}
 		}
 

test/e2e/create_test.go

@@ -461,11 +461,15 @@ var _ = Describe("Podman create", func() {
 	It("podman create --tz", func() {
 		session := podmanTest.Podman([]string{"create", "--tz", "foo", "--name", "bad", ALPINE, "date"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError())
+		Expect(session).To(Exit(125))
+		Expect(session.ErrorToString()).To(
+			Equal("Error: running container create option: finding timezone: unknown time zone foo"))
 
 		session = podmanTest.Podman([]string{"create", "--tz", "America", "--name", "dir", ALPINE, "date"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError())
+		Expect(session).To(Exit(125))
+		Expect(session.ErrorToString()).To(
+			Equal("Error: running container create option: finding timezone: is a directory"))
 
 		session = podmanTest.Podman([]string{"create", "--tz", "Pacific/Honolulu", "--name", "zone", ALPINE, "date"})
 		session.WaitWithDefaultTimeout()

test/e2e/run_test.go

@@ -1611,6 +1611,7 @@ USER mail`, BB)
 		tzFile := filepath.Join(testDir, "tzfile.txt")
 		file, err := os.Create(tzFile)
 		Expect(err).ToNot(HaveOccurred())
+		defer os.Remove(tzFile)
 
 		_, err = file.WriteString("Hello")
 		Expect(err).ToNot(HaveOccurred())
@@ -1620,18 +1621,8 @@ USER mail`, BB)
 		session := podmanTest.Podman([]string{"run", "--tz", badTZFile, "--rm", ALPINE, "date"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).To(ExitWithError())
-		Expect(session.ErrorToString()).To(ContainSubstring("finding timezone for container"))
-
-		err = os.Remove(tzFile)
-		Expect(err).ToNot(HaveOccurred())
-
-		session = podmanTest.Podman([]string{"run", "--tz", "foo", "--rm", ALPINE, "date"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError())
-
-		session = podmanTest.Podman([]string{"run", "--tz", "America", "--rm", ALPINE, "date"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError())
+		Expect(session.ErrorToString()).To(
+			Equal("Error: running container create option: finding timezone: time: invalid location name"))
 
 		session = podmanTest.Podman([]string{"run", "--tz", "Pacific/Honolulu", "--rm", ALPINE, "date", "+'%H %Z'"})
 		session.WaitWithDefaultTimeout()

test/system/030-run.bats

@@ -472,6 +472,14 @@ json-file | f
     is "$output" "$expect" "podman run with --tz=local, matches host"
 }
 
+@test "podman run --tz with zoneinfo" {
+    # First make sure that zoneinfo is actually in the image otherwise the test is pointless
+    run_podman run --rm $SYSTEMD_IMAGE ls /usr/share/zoneinfo
+
+    run_podman run --rm --tz Europe/Berlin $SYSTEMD_IMAGE readlink /etc/localtime
+    assert "$output" == "../usr/share/zoneinfo/Europe/Berlin" "localtime is linked correctly"
+}
+
 # run with --runtime should preserve the named runtime
 @test "podman run : full path to --runtime is preserved" {
     skip_if_remote "podman-remote does not support --runtime option"

test/system/build-systemd-image

@@ -31,10 +31,11 @@ cd $tmpdir
 echo $YMD >testimage-id
 
 cat >Containerfile <<EOF
-FROM registry.fedoraproject.org/fedora-minimal:37
+FROM registry.fedoraproject.org/fedora-minimal:38
 LABEL created_by="$create_script @ $create_script_rev"
 LABEL created_at=$create_time_z
-RUN microdnf install -y systemd && microdnf clean all && sed -i -e 's/.*LogColor.*/LogColor=no/' /etc/systemd/system.conf
+# Note the reinstall of tzdata is required (https://bugzilla.redhat.com/show_bug.cgi?id=1870814)
+RUN microdnf install -y systemd && microdnf reinstall -y tzdata && microdnf clean all && sed -i -e 's/.*LogColor.*/LogColor=no/' /etc/systemd/system.conf
 ADD testimage-id /home/podman/
 WORKDIR /home/podman
 CMD ["/bin/echo", "This image is intended for podman CI testing"]

test/system/helpers.bash

@@ -14,7 +14,7 @@ PODMAN_TEST_IMAGE_ID=
 
 # Larger image containing systemd tools.
 PODMAN_SYSTEMD_IMAGE_NAME=${PODMAN_SYSTEMD_IMAGE_NAME:-"systemd-image"}
-PODMAN_SYSTEMD_IMAGE_TAG=${PODMAN_SYSTEMD_IMAGE_TAG:-"20230106"}
+PODMAN_SYSTEMD_IMAGE_TAG=${PODMAN_SYSTEMD_IMAGE_TAG:-"20230531"}
 PODMAN_SYSTEMD_IMAGE_FQN="$PODMAN_TEST_IMAGE_REGISTRY/$PODMAN_TEST_IMAGE_USER/$PODMAN_SYSTEMD_IMAGE_NAME:$PODMAN_SYSTEMD_IMAGE_TAG"
 
 # Remote image that we *DO NOT* fetch or keep by default; used for testing pull