From 059eed7d622ea94aa6aae9148a3c185bb6aa536a Mon Sep 17 00:00:00 2001 From: jamesread Date: Tue, 28 Jul 2026 13:58:00 +0100 Subject: [PATCH] chore: coderabbit suggestions --- .../pages/reference/multiple_instances.adoc | 2 +- .../ROOT/pages/reference/network-ports.adoc | 2 +- frontend/resources/vue/views/LogsListView.vue | 33 ++++++++--- service/internal/config/config_reloader.go | 56 +++++++++++++------ service/internal/config/port_env_test.go | 20 +++++++ 5 files changed, 86 insertions(+), 27 deletions(-) diff --git a/docs/modules/ROOT/pages/reference/multiple_instances.adoc b/docs/modules/ROOT/pages/reference/multiple_instances.adoc index f8935b2..db1c8f9 100644 --- a/docs/modules/ROOT/pages/reference/multiple_instances.adoc +++ b/docs/modules/ROOT/pages/reference/multiple_instances.adoc @@ -28,7 +28,7 @@ When you come to create the config.yaml file, OliveTin will look for this in it' Because you are running outside of a container, you will also need to change the "internal" ports used by OliveTin so they are separate for all instances. OliveTin listens on 4 addresses (1 external, 3 internal) and needs 4 ports. You can read about these in the xref:reference/network-ports.adoc[network ports documentation]. -NOTE: If the `PORT` environment variable is set, OliveTin listens on that port for the single HTTP frontend (overriding `listenAddressSingleHTTPFrontend` in the config). When `PORT` is unset, the config value is used, or `1337` if that setting is omitted. `PORT` is also used as a base for the other internal listen addresses (+1, +2, …) when those are not set in the config. For example, if `PORT` is 2000, the single frontend starts on port 2000, the REST API on 2001, and so on. +NOTE: If the `PORT` environment variable is set, OliveTin listens on that port for the single HTTP frontend only (overriding `listenAddressSingleHTTPFrontend` in the config, keeping the host from the config). When `PORT` is unset, the config value is used, or `0.0.0.0:1337` if that setting is omitted. `PORT` does not change the other internal listen addresses — set those explicitly in each instance's config as shown below. You could end up with a setup that looks like this; diff --git a/docs/modules/ROOT/pages/reference/network-ports.adoc b/docs/modules/ROOT/pages/reference/network-ports.adoc index 813960d..e96abb4 100644 --- a/docs/modules/ROOT/pages/reference/network-ports.adoc +++ b/docs/modules/ROOT/pages/reference/network-ports.adoc @@ -53,7 +53,7 @@ Below is a detailed reference table. == PORT environment variable -If the `PORT` environment variable is set at startup, OliveTin uses it as the listen port for `listenAddressSingleHTTPFrontend`, keeping the host from the config (default host `0.0.0.0`). This overrides an explicit port in `config.yaml`, which is useful on platforms that assign a port via `PORT` (for example Heroku or Cloud Run). +If the `PORT` environment variable is set at startup, OliveTin uses it as the listen port for `listenAddressSingleHTTPFrontend` only, keeping the host from the config (default host `0.0.0.0`). This overrides an explicit port in `config.yaml`, which is useful on platforms that assign a port via `PORT` (for example Heroku or Cloud Run). Internal listen addresses (`listenAddressRestActions`, `listenAddressWebUI`, and so on) are not derived from `PORT`; configure those separately when needed. When `PORT` is not set, OliveTin uses `listenAddressSingleHTTPFrontend` from the config, or `0.0.0.0:1337` if that setting is omitted. diff --git a/frontend/resources/vue/views/LogsListView.vue b/frontend/resources/vue/views/LogsListView.vue index 940703b..bbeeaaf 100644 --- a/frontend/resources/vue/views/LogsListView.vue +++ b/frontend/resources/vue/views/LogsListView.vue @@ -275,9 +275,34 @@ watch(() => route.query.date, () => { watch(searchText, (value) => { currentPage.value = 1 storeLogsFilter(value) + syncFilterToRoute(value) scheduleFetchLogs() }) +watch(() => route.query.filter, (filter) => { + const next = typeof filter === 'string' ? filter : '' + if (searchText.value === next) { + return + } + searchText.value = next +}) + +function syncFilterToRoute (value) { + const next = value || '' + const current = typeof route.query.filter === 'string' ? route.query.filter : '' + if (next === current) { + return + } + + const query = { ...route.query } + if (next) { + query.filter = next + } else { + delete query.filter + } + router.replace({ path: route.path, query }) +} + async function fetchLogs () { loading.value = true filterError.value = '' @@ -326,14 +351,6 @@ function scheduleFetchLogs () { function clearSearch () { searchText.value = '' - - if (route.query.filter == null) { - return - } - - const query = { ...route.query } - delete query.filter - router.replace({ path: route.path, query }) } function clearDateFilter () { diff --git a/service/internal/config/config_reloader.go b/service/internal/config/config_reloader.go index a3f92be..9b85c6d 100644 --- a/service/internal/config/config_reloader.go +++ b/service/internal/config/config_reloader.go @@ -90,27 +90,28 @@ func afterLoadFinalize(cfg *Config, configPath string) { } } -// applyPortEnvironmentOverride sets the single HTTP frontend listen port from -// $PORT when that environment variable is set. This runs after config unmarshal -// so PORT wins over listenAddressSingleHTTPFrontend in config.yaml (common on -// Heroku, Cloud Run, and similar hosts). When PORT is unset, the config value -// or the default (1337) is left unchanged. +// applyPortEnvironmentOverride lets the PORT environment variable take precedence +// over the configured HTTP frontend port. func applyPortEnvironmentOverride(cfg *Config) { envPort := strings.TrimSpace(os.Getenv("PORT")) if envPort == "" { return } - port, err := strconv.Atoi(envPort) - if err != nil || port < 1 || port > 65535 { - log.WithFields(log.Fields{ - "PORT": envPort, - "error": err, - }).Error("Ignoring invalid PORT environment variable") + port, ok := parseEnvPort(envPort) + if !ok { + return + } + + host, ok := listenHostOrDefault(cfg.ListenAddressSingleHTTPFrontend) + if !ok { + log.WithFields(log.Fields{ + "PORT": envPort, + "listenAddress": cfg.ListenAddressSingleHTTPFrontend, + }).Error("Ignoring PORT environment variable because listenAddressSingleHTTPFrontend is invalid") return } - host := listenHostOrDefault(cfg.ListenAddressSingleHTTPFrontend) cfg.ListenAddressSingleHTTPFrontend = net.JoinHostPort(host, strconv.Itoa(port)) log.WithFields(log.Fields{ @@ -118,13 +119,34 @@ func applyPortEnvironmentOverride(cfg *Config) { }).Info("Using PORT environment variable for single HTTP frontend listen address") } -func listenHostOrDefault(listenAddress string) string { - host, _, err := net.SplitHostPort(listenAddress) - if err != nil || host == "" { - return "0.0.0.0" +func parseEnvPort(envPort string) (int, bool) { + port, err := strconv.Atoi(envPort) + if err != nil || port < 1 || port > 65535 { + log.WithFields(log.Fields{ + "PORT": envPort, + "error": err, + }).Error("Ignoring invalid PORT environment variable") + return 0, false } - return host + return port, true +} + +func listenHostOrDefault(listenAddress string) (string, bool) { + if strings.TrimSpace(listenAddress) == "" { + return "0.0.0.0", true + } + + host, _, err := net.SplitHostPort(listenAddress) + if err != nil { + return "", false + } + + if host == "" { + return "0.0.0.0", true + } + + return host, true } // buildIncludePath constructs the full path to the include directory. diff --git a/service/internal/config/port_env_test.go b/service/internal/config/port_env_test.go index e4034b8..4c938c2 100644 --- a/service/internal/config/port_env_test.go +++ b/service/internal/config/port_env_test.go @@ -45,3 +45,23 @@ func TestApplyPortEnvironmentOverrideIgnoresInvalid(t *testing.T) { assert.Equal(t, "0.0.0.0:1337", cfg.ListenAddressSingleHTTPFrontend) } + +func TestApplyPortEnvironmentOverrideEmptyListenAddressDefaultsHost(t *testing.T) { + t.Setenv("PORT", "8080") + + cfg := DefaultConfig() + cfg.ListenAddressSingleHTTPFrontend = "" + applyPortEnvironmentOverride(cfg) + + assert.Equal(t, "0.0.0.0:8080", cfg.ListenAddressSingleHTTPFrontend) +} + +func TestApplyPortEnvironmentOverrideRejectsMalformedListenAddress(t *testing.T) { + t.Setenv("PORT", "8080") + + cfg := DefaultConfig() + cfg.ListenAddressSingleHTTPFrontend = "not-a-valid-address" + applyPortEnvironmentOverride(cfg) + + assert.Equal(t, "not-a-valid-address", cfg.ListenAddressSingleHTTPFrontend) +}