Repository: ddev/ddev
Pull request #6603 (2024-10-09 16:52:51)

fix: improve timezone detection, for #6468
## The Issue

While attending today's contributor training on `ddev debug test`, I noticed a problem with timezone detection on Randy's MBP:

```
$ DDEV_DEBUG=true ddev start -y
...
2024-10-09T07:42:08.796 Unable to autodetect timezone: unable to read timezone from /usr/share/zoneinfo.default/America/Denver file
...
```

- #6468

## How This PR Solves The Issue

After research, we found that `filepath.EvalSymlinks()` evaluates hidden symlinks on macOS:

```
$ ls -l /etc/localtime
lrwxr-xr-x  1 root  wheel  40 Oct  6 13:46 /etc/localtime -> /var/db/timezone/zoneinfo/America/Denver
```

```
$ ls -ltd /var/db/timezone/zoneinfo
lrwxr-xr-x  1 root  wheel  38 May 21 14:48 /var/db/timezone/zoneinfo -> /var/db/timezone/tz/2024a.1.0/zoneinfo
```

```
$ ls -ltd /var/db/timezone/zoneinfo
lrwxr-xr-x  1 root  wheel  27 Oct  6 13:45 /var/db/timezone/zoneinfo -> /usr/share/zoneinfo.default
```

---

And I noticed my own comment https://github.com/ddev/ddev/pull/6468#issuecomment-2273287479
> TODO remove optional right/ and posix/ from the timezone symlink https://stackoverflow.com/q/67885319/8097891

Which I forgot about, so I added it in this PR.

## Manual Testing Instructions

Run `DDEV_DEBUG=true ddev start` on macOS Sequoia.

The output should have this:
```
Using automatically detected timezone: TZ=America/Denver
```

And `ddev exec 'echo $TZ'` should show an appropriate timezone from the host.

## Automated Testing Overview

<!-- Please describe the tests introduced by this PR, or explain why no tests are needed. -->

## Release/Deployment Notes

<!-- Does this affect anything else or have ramifications for other code? Does anything have to be done on deployment? -->


github-actions[bot]: <!-- bot: nightly-link -->
Download the artifacts for this pull request:

* [all-ddev-executables.zip](https://nightly.link/ddev/ddev/actions/artifacts/2036165088.zip)
* [ddev-macos-amd64.zip](https://nightly.link/ddev/ddev/actions/artifacts/2036165198.zip)
* [ddev-macos-arm64.zip](https://nightly.link/ddev/ddev/actions/artifacts/2036165294.zip)
* [ddev-linux-arm64.zip](https://nightly.link/ddev/ddev/actions/artifacts/2036165371.zip)
* [ddev-linux-amd64.zip](https://nightly.link/ddev/ddev/actions/artifacts/2036165456.zip)
* [ddev-windows-amd64.zip](https://nightly.link/ddev/ddev/actions/artifacts/2036165554.zip)
* [ddev-windows-amd64-installer.zip](https://nightly.link/ddev/ddev/actions/artifacts/2036165636.zip)
* [ddev-windows-arm64.zip](https://nightly.link/ddev/ddev/actions/artifacts/2036165719.zip)
* [ddev-windows-arm64-installer.zip](https://nightly.link/ddev/ddev/actions/artifacts/2036165786.zip)

See [Testing a PR](https://ddev.readthedocs.io/en/latest/developers/building-contributing/#testing-a-pr).

[![Open in Gitpod](https://gitpod.io/button/open-in-gitpod.svg)](https://gitpod.io/#https://github.com/ddev/ddev/pull/6603)

[![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://github.com/codespaces/new?ref=20241009_stasadev_timezone&repo=669738338)
stasadev: We have E2E testing here, I rely on what `/etc/localtime` returns.

I didn't find this path in the test runners, so it was unexpected for me to see `zoneinfo.default`

I could probably add unit testing here, but that would require moving some logic to a new function. Is it reasonable to do this?

Patch:
diff --git a/pkg/ddevapp/ddevapp.go b/pkg/ddevapp/ddevapp.go
index 3ff2ff5fa..9dbd0d3d7 100644
--- a/pkg/ddevapp/ddevapp.go
+++ b/pkg/ddevapp/ddevapp.go
@@ -1187,28 +1187,13 @@ func (app *DdevApp) GetLocalTimezone() (string, error) {
 		timezone = os.Getenv("TZ")
 	} else {
 		localtimeFile := filepath.Join("/etc", "localtime")
-		timezoneFile, err := filepath.EvalSymlinks(localtimeFile)
+		var err error
+		timezone, err = filepath.EvalSymlinks(localtimeFile)
 		if err != nil {
 			return "", fmt.Errorf("unable to read timezone from %s file: %v", localtimeFile, err)
 		}
-		// /etc/localtime is a symlink to a file, for example:
-		// /usr/share/zoneinfo/Europe/London on Linux and WSL2
-		// /var/db/timezone/zoneinfo/Europe/London on macOS (the exact path to /zoneinfo/ may differ)
-		// We can search for anything after /zoneinfo/ in the file path
-		parts := strings.Split(strings.TrimSpace(timezoneFile), "/zoneinfo/")
-		if len(parts) != 2 {
-			return "", fmt.Errorf("unable to read timezone from %s file", timezoneFile)
-		}
-		timezone = parts[1]
-		if timezone == "" {
-			return "", fmt.Errorf("unable to read timezone from %s file", timezoneFile)
-		}
-	}
-	_, err := time.LoadLocation(timezone)
-	if err != nil {
-		return "", fmt.Errorf("failed to load timezone '%s': %v", timezone, err)
 	}
-	return timezone, nil
+	return util.GetTimezone(timezone)
 }
 
 // Start initiates docker-compose up
diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index 19deba4e7..42797481d 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -9,6 +9,7 @@ import (
 	osexec "os/exec"
 	"os/user"
 	"path/filepath"
+	"regexp"
 	"runtime"
 	"strings"
 	"time"
@@ -388,3 +389,33 @@ func Chmod(path string, mode os.FileMode) error {
 	}
 	return os.Chmod(path, mode)
 }
+
+// GetTimezone tries to find local timezone from the path, that can be
+// either $TZ environment variable or /etc/localtime symlink
+func GetTimezone(path string) (string, error) {
+	// Use case-insensitive search for /zoneinfo/ in the file path.
+	regex := regexp.MustCompile(`(?i)/.*?zoneinfo.*?/`)
+	parts := regex.Split(strings.TrimSpace(path), 2)
+	if len(parts) != 2 {
+		// If this is not a path, but timezone, return it.
+		_, err := time.LoadLocation(path)
+		if err == nil {
+			return path, nil
+		}
+		return "", fmt.Errorf("unable to read timezone from %s", path)
+	}
+	timezone := parts[1]
+	// Remove leading prefixes if they exist.
+	// https://stackoverflow.com/a/67888343/8097891
+	for _, prefix := range []string{"posix/", "right/"} {
+		timezone = strings.TrimPrefix(timezone, prefix)
+	}
+	if timezone == "" {
+		return "", fmt.Errorf("unable to read timezone from %s", path)
+	}
+	_, err := time.LoadLocation(timezone)
+	if err != nil {
+		return "", fmt.Errorf("failed to load timezone '%s': %v", timezone, err)
+	}
+	return timezone, nil
+}


Test patch:
diff --git a/docs/content/developers/buildkite-testmachine-setup.md b/docs/content/developers/buildkite-testmachine-setup.md
index 471be36a7..55a60023a 100644
--- a/docs/content/developers/buildkite-testmachine-setup.md
+++ b/docs/content/developers/buildkite-testmachine-setup.md
@@ -60,24 +60,16 @@ We are using [Buildkite](https://buildkite.com/ddev) for Windows and macOS testi
 13. `sudo systemctl enable buildkite-agent && sudo systemctl start buildkite-agent`
 14. In PowerShell: `wsl.exe --update`. Watch for the escalation to complete, it may require escalation.
 15. Open WSL2 and check out [ddev/ddev](https://github.com/ddev/ddev).
-16. Install Homebrew: `/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"`
-17. Configure brew in PATH with:
-
-     ```
-     echo 'export PATH="/snap/bin:/home/linuxbrew/.linuxbrew/bin:$PATH"' >>~/.bashrc
-     source ~/.bashrc
-     ```
-
-18. As root user (`sudo -s`), add sudo capability with `echo "ALL ALL=NOPASSWD: ALL" >/etc/sudoers.d/all && chmod 440 /etc/sudoers.d/all`.
-19. `git config --global --add safe.directory '*'`
-20. `echo "capath=/etc/ssl/certs/" >>~/.curlrc`
-21. `nc.exe -L -p 9003` on Windows to trigger and allow Windows Defender.
-22. Run `ngrok authtoken <token>` with token for free account.
-23. Install [winaero tweaker](https://winaero.com/request.php?1796) and “Enable user autologin checkbox”. Set up the machine to [automatically log in on boot](https://www.cnet.com/how-to/automatically-log-in-to-your-windows-10-pc/).  Then run netplwiz, provide the password for the main user, uncheck “require a password to log in”.
-24. In “Advanced Windows Update Settings” enable “Receive updates for other Microsoft products” to make sure you get WSL2 kernel upgrades. Make sure to run Windows Update to get the latest kernel.
-25. Turn off the `System` -> `Notifications` -> `Additional settings` that cause the "Windows experience" prompts after upgrades:
+16. As root user (`sudo -s`), add sudo capability with `echo "ALL ALL=NOPASSWD: ALL" >/etc/sudoers.d/all && chmod 440 /etc/sudoers.d/all`.
+17. `git config --global --add safe.directory '*'`
+18. `echo "capath=/etc/ssl/certs/" >>~/.curlrc`
+19. `nc.exe -L -p 9003` on Windows to trigger and allow Windows Defender.
+20. Run `ngrok authtoken <token>` with token for free account.
+21. Install [winaero tweaker](https://winaero.com/request.php?1796) and “Enable user autologin checkbox”. Set up the machine to [automatically log in on boot](https://www.cnet.com/how-to/automatically-log-in-to-your-windows-10-pc/).  Then run netplwiz, provide the password for the main user, uncheck “require a password to log in”.
+22. In “Advanced Windows Update Settings” enable “Receive updates for other Microsoft products” to make sure you get WSL2 kernel upgrades. Make sure to run Windows Update to get the latest kernel.
+23. Turn off the `System` -> `Notifications` -> `Additional settings` that cause the "Windows experience" prompts after upgrades:
     ![disable_windows_experience](../images/disable_windows_experience.png)
-26. Edit the `~/.wslconfig` on Windows to add appropriate WSL2 memory allocation and `autoMemoryReclaim`
+24. Edit the `~/.wslconfig` on Windows to add appropriate WSL2 memory allocation and `autoMemoryReclaim`
 
     ```
     memory=12GB
diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go
index 041a28851..8139b6258 100644
--- a/pkg/util/utils_test.go
+++ b/pkg/util/utils_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/ddev/ddev/pkg/output"
 	"github.com/ddev/ddev/pkg/util"
 	asrt "github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // TestRandString ensures that RandString only generates string of the correct value and characters.
@@ -194,3 +195,38 @@ func TestArrayToReadableOutput(t *testing.T) {
 	_, err := util.ArrayToReadableOutput([]string{})
 	assert.EqualErrorf(err, "empty slice", "Expected error when passing an empty slice")
 }
+
+// TestGetTimezone tests GetTimezone
+func TestGetTimezone(t *testing.T) {
+	testCases := []struct {
+		description string
+		input       string
+		result      string
+		error       string
+	}{
+		{"$TZ env var", "Europe/London", "Europe/London", ""},
+		{"Linux /etc/localtime symlink", "/usr/share/zoneinfo/Europe/London", "Europe/London", ""},
+		{"Linux /etc/localtime posix symlink", "/usr/share/zoneinfo/posix/Europe/London", "Europe/London", ""},
+		{"Linux /etc/localtime right symlink", "/usr/share/zoneinfo/right/Europe/London", "Europe/London", ""},
+		{"/etc/localtime is not a symlink", "/etc/localtime", "", "unable to read timezone from /etc/localtime"},
+		{"macOS /etc/localtime symlink", "/var/db/timezone/zoneinfo/Europe/London", "Europe/London", ""},
+		{"macOS Sonoma /etc/localtime symlink", "/private/var/db/timezone/tz/2024a.1.0/zoneinfo/Europe/London", "Europe/London", ""},
+		{"macOS Sequoia /etc/localtime symlink", "/usr/share/zoneinfo.default/Europe/London", "Europe/London", ""},
+		{"Case-insensitive search for /zoneinfo/ in the path", "/path/to/TestZoneInfoTest/Europe/London", "Europe/London", ""},
+		{"Timezone has wrong format", "/Europe/London", "", "unable to read timezone from /Europe/London"},
+		{"Not real timezone", "Europe/Test", "", "unable to read timezone from Europe/Test"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			timezone, err := util.GetTimezone(tc.input)
+			require.Equal(t, tc.result, timezone)
+			if tc.error == "" {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), tc.error)
+			}
+		})
+	}
+}