From 7fd473b88b1fc38ef4a9598fe705657f633a9545 Mon Sep 17 00:00:00 2001 From: mhoennig Date: Fri, 4 Sep 2026 17:47:16 +0200 Subject: [PATCH 1/3] Warn instead of silently defaulting when init cannot load the config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `init --systemd` read the effective configuration through a catch-all that swallowed every exception and continued with a default `ServerConfig`. A repository whose `.werkator.yml` failed validation for any reason then looked exactly like one with no `publicBaseUrl`: the `.htaccess` and the maintenance page were skipped without a word, during initial deployment setup of all times. The fallback stays — the units are still generated — but the exception message is now printed. The configuration is loaded once per run so the three readers inside `createSystemdFiles` do not repeat the warning. Co-Authored-By: Claude Opus 5 --- .../hoennig/werkator/commands/InitCommand.kt | 23 +++++++++++++++---- .../werkator/commands/InitCommandTest.kt | 22 ++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/main/kotlin/de/hoennig/werkator/commands/InitCommand.kt b/src/main/kotlin/de/hoennig/werkator/commands/InitCommand.kt index f7a8616..a824341 100644 --- a/src/main/kotlin/de/hoennig/werkator/commands/InitCommand.kt +++ b/src/main/kotlin/de/hoennig/werkator/commands/InitCommand.kt @@ -48,6 +48,7 @@ class InitCommand( internal var javaExecutableResolver: () -> Path = { Paths.get(System.getProperty("java.home"), "bin", "java") } override fun run() { + cachedServerConfig = null val normalizedWorkingDir = workingDir.toAbsolutePath().normalize() val root = try { @@ -298,12 +299,24 @@ class InitCommand( */ private fun loadedSystemdConfig(): de.hoennig.werkator.config.SystemdConfig = loadedServerConfig().systemd + /** Loaded once per run, so a broken configuration is reported once and not per caller. */ + private var cachedServerConfig: de.hoennig.werkator.config.ServerConfig? = null + + /** + * A configuration error here is not fatal — the units are still generated with defaults — + * but it must not pass for "nothing configured": without the warning a broken `.werkator.yml` + * looks exactly like an unset `publicBaseUrl` and the host integration is skipped silently. + */ private fun loadedServerConfig(): de.hoennig.werkator.config.ServerConfig = - try { - configLoader.load(Paths.get(".")).server - } catch (_: Exception) { - de.hoennig.werkator.config - .ServerConfig() + cachedServerConfig ?: run { + try { + configLoader.load(Paths.get(".")).server + } catch (e: Exception) { + println("Warning: the effective configuration could not be loaded (${e.message})") + println(" continuing with default server settings — check the generated unit and host files") + de.hoennig.werkator.config + .ServerConfig() + }.also { cachedServerConfig = it } } private fun createSystemdFiles( diff --git a/src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt b/src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt index 14ae12d..2d16448 100644 --- a/src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt @@ -252,5 +252,27 @@ class InitCommandTest : FunSpec() { // This should not throw IllegalArgumentException initCommand.run() } + + test("--systemd warns once when the effective configuration cannot be loaded") { + val tempDir = Files.createTempDirectory("werkator-init-test") + val brokenLoader = mockk() + every { brokenLoader.load(any()) } throws IllegalStateException("gitea.owner is required") + val command = InitCommand(gitService, brokenLoader) + command.workingDir = tempDir + command.systemd = true + command.jarPathResolver = { Paths.get("/home/ci/bin/werkator.jar") } + command.javaExecutableResolver = { Paths.get("/usr/bin/java") } + + every { gitService.getTopLevel(tempDir) } returns tempDir + every { gitService.getOriginUrl(tempDir) } returns "https://git.example.org/my-org/my-repo.git" + + val console = captureConsole { command.run() } + + console.stdout shouldContain "gitea.owner is required" + // the three readers of the configuration must not repeat the warning + console.stdout.windowed("Warning:".length).count { it == "Warning:" } shouldBe 1 + // the units are still written with the defaults + tempDir.resolve(".git/werkator/${SystemdServiceFiles.unitName(tempDir)}").toFile().shouldExist() + } } } -- 2.39.5 From 2cddd76c8871bb96f0872a9cc7337ccbfd6730f5 Mon Sep 17 00:00:00 2001 From: mhoennig Date: Fri, 4 Sep 2026 17:50:58 +0200 Subject: [PATCH 2/3] Read init's effective configuration from the repository root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `init --systemd` resolved the configuration through `Paths.get(".")` while every other file it touches — the two generated config layers, an applied fragment, the units, the .htaccess — goes through the git top level. The two agree only when the process happens to run in the repository root; from a subdirectory the command silently read a different repository's configuration or none at all, so `--apply`'s promise that the fragment's port reaches the generated unit did not hold, and the host integration was skipped as if no publicBaseUrl were configured. The root is passed down instead, matching every other caller of `ConfigLoader.load`. Co-Authored-By: Claude Opus 5 --- .../hoennig/werkator/commands/InitCommand.kt | 17 +++++++++----- .../werkator/commands/InitCommandTest.kt | 23 +++++++++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/main/kotlin/de/hoennig/werkator/commands/InitCommand.kt b/src/main/kotlin/de/hoennig/werkator/commands/InitCommand.kt index a824341..f3c86f7 100644 --- a/src/main/kotlin/de/hoennig/werkator/commands/InitCommand.kt +++ b/src/main/kotlin/de/hoennig/werkator/commands/InitCommand.kt @@ -297,20 +297,25 @@ class InitCommand( * already loadable (re-running `init --systemd` on an installed instance); during * the very first bootstrap they stay unset and the defaults (no directives) apply. */ - private fun loadedSystemdConfig(): de.hoennig.werkator.config.SystemdConfig = loadedServerConfig().systemd + private fun loadedSystemdConfig(root: Path): de.hoennig.werkator.config.SystemdConfig = loadedServerConfig(root).systemd /** Loaded once per run, so a broken configuration is reported once and not per caller. */ private var cachedServerConfig: de.hoennig.werkator.config.ServerConfig? = null /** + * Read from the repository root like every other file this command touches — the + * layers sit there, not in whatever directory the process happens to run in, and + * an applied fragment must reach the generated unit even when `init` is invoked + * from a subdirectory. + * * A configuration error here is not fatal — the units are still generated with defaults — * but it must not pass for "nothing configured": without the warning a broken `.werkator.yml` * looks exactly like an unset `publicBaseUrl` and the host integration is skipped silently. */ - private fun loadedServerConfig(): de.hoennig.werkator.config.ServerConfig = + private fun loadedServerConfig(root: Path): de.hoennig.werkator.config.ServerConfig = cachedServerConfig ?: run { try { - configLoader.load(Paths.get(".")).server + configLoader.load(root).server } catch (e: Exception) { println("Warning: the effective configuration could not be loaded (${e.message})") println(" continuing with default server settings — check the generated unit and host files") @@ -340,8 +345,8 @@ class InitCommand( javaExecutable = javaExecutableResolver(), jarPath = jarPath, envFile = envFile, - memoryMax = loadedSystemdConfig().memoryMax, - tasksMax = loadedSystemdConfig().tasksMax, + memoryMax = loadedSystemdConfig(root).memoryMax, + tasksMax = loadedSystemdConfig(root).tasksMax, ), ) println("created ${unitFile.toFile().relativeTo(normalizedWorkingDir.toFile())}") @@ -364,7 +369,7 @@ class InitCommand( // generated host integration like the units: only meaningful behind a web // frontend, so it needs a public base URL; unused elsewhere and harmless - val server = loadedServerConfig() + val server = loadedServerConfig(root) if (server.publicBaseUrl.isNotBlank()) { val htaccessFile = werkatorDir.resolve(SystemdServiceFiles.HTACCESS_NAME) htaccessFile.toFile().writeText(SystemdServiceFiles.htaccessContent(server.port)) diff --git a/src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt b/src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt index 2d16448..a2f289f 100644 --- a/src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt @@ -253,6 +253,29 @@ class InitCommandTest : FunSpec() { initCommand.run() } + test("--systemd reads the configuration from the repository root, not the current directory") { + val tempDir = Files.createTempDirectory("werkator-init-test") + // written before the run, so `init` keeps it instead of creating a template + tempDir.resolve(".werkator.yml").toFile().writeText( + "server:\n publicBaseUrl: \"https://werkator.example.org/\"\n port: 18099\n", + ) + initCommand.workingDir = tempDir + initCommand.systemd = true + initCommand.jarPathResolver = { Paths.get("/home/ci/bin/werkator.jar") } + initCommand.javaExecutableResolver = { Paths.get("/usr/bin/java") } + + every { gitService.getTopLevel(tempDir) } returns tempDir + every { gitService.getOriginUrl(tempDir) } returns "https://git.example.org/my-org/my-repo.git" + + initCommand.run() + + // the host integration is generated only when the root's config has a public base URL + val htaccess = tempDir.resolve(".git/werkator/${SystemdServiceFiles.HTACCESS_NAME}") + htaccess.toFile().shouldExist() + htaccess.toFile().readText() shouldContain "18099" + tempDir.resolve(".git/werkator/${SystemdServiceFiles.MAINTENANCE_PAGE_NAME}").toFile().shouldExist() + } + test("--systemd warns once when the effective configuration cannot be loaded") { val tempDir = Files.createTempDirectory("werkator-init-test") val brokenLoader = mockk() -- 2.39.5 From fb1db30f2aeb52737a2dc0e0614cf87f2824ba83 Mon Sep 17 00:00:00 2001 From: mhoennig Date: Fri, 4 Sep 2026 18:02:58 +0200 Subject: [PATCH 3/3] docs(prs): the init-config-resolution PR-doc Co-Authored-By: Claude Opus 5 --- ...2026-09-04-PR#22-init-config-resolution.md | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 docs/prs/2026-09-04-PR#22-init-config-resolution.md diff --git a/docs/prs/2026-09-04-PR#22-init-config-resolution.md b/docs/prs/2026-09-04-PR#22-init-config-resolution.md new file mode 100644 index 0000000..043bacf --- /dev/null +++ b/docs/prs/2026-09-04-PR#22-init-config-resolution.md @@ -0,0 +1,90 @@ +> **WARNING:** This document describes only the change applied in this PR. +> It may already be outdated once the next PR is merged. +> Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation. + +## The Problem + +`init --systemd` generates the host integration — the systemd unit's resource limits, the Apache `.htaccess` and the maintenance page — from the effective configuration. +It read that configuration through a helper with two independent defects, both of which fail silently. + +**It swallowed every error.** +The load sat in `try { … } catch (_: Exception) { ServerConfig() }`. +Any configuration error at all — a missing required field, a malformed layer, a version floor violation — produced a default `ServerConfig` with a blank `publicBaseUrl`. +A repository with a broken `.werkator.yml` then looked exactly like one that simply has no public base URL configured: +the `.htaccess` and the maintenance page were skipped without a word. +This surfaced while verifying [PR#17](2026-09-03-PR%2317-maintenance-page.md) on mih09, where the missing files looked like an unconfigured `publicBaseUrl` and were in fact an unrelated validation error. + +**It read from the wrong directory.** +The helper called `configLoader.load(Paths.get("."))` — the process's current directory — while everything else in the command works off the git top level resolved by `GitService.getTopLevel`. +`ConfigLoader.loadRaw` resolves the layers directly under the directory it is given and does not walk up to the repository root, so the two agree only when `init` happens to be invoked from the root itself. +From a subdirectory the command read another repository's configuration, or none. +That also broke `--apply`: the fragment is installed into the repository root deliberately before the systemd files are written, so that its port and limits reach the generated unit, and a current-directory read does not see it. + +`init --systemd` runs during initial deployment setup, which is exactly when a silent wrong answer is most expensive. + +## Non-Goals + +- The fallback itself is kept: a configuration that cannot be loaded is not fatal for `init`, the units are still generated with the defaults. + During the very first bootstrap there is legitimately nothing to load yet. +- No change to `ConfigLoader`, to the configuration schema, or to any other command. +- No sweep for catch-all exception handlers elsewhere in the code base; + the two other `catch` blocks in `InitCommand` already print an `Error:` and abort, so they were only checked, not changed. + +## The Scenarios + +### Feature: init reports what it read and where it read it from + +#### Background + +- The *repository root* is the git top level as resolved by `GitService.getTopLevel`, the directory holding `.werkator.yml`, `.git/werkator/.werkator.yml` and an applied fragment. +- The *current directory* is the process working directory, which is the repository root only when `init` is invoked there. + +#### Scenario#22.01: A broken configuration is named, not defaulted over + +So that a validation error during deployment setup is not mistaken for an unconfigured installation. + +- **Given** a repository whose effective configuration cannot be loaded +- **When** `init --systemd` runs +- **Then** the exception message is printed as a warning + - **and** the unit files are still generated with the default settings + - **and** the warning appears exactly once, although three settings are read from the configuration + +##### Verified by + +- [InitCommandTest: `--systemd warns once when the effective configuration cannot be loaded`](../../src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt) + +#### Scenario#22.02: The configuration is read from the repository root + +So that the generated host integration reflects the repository being initialized, whatever directory `init` was invoked from. + +- **Given** a repository whose root configuration sets `server.publicBaseUrl` and `server.port` +- **and** a current directory that is not that repository root +- **When** `init --systemd` runs +- **Then** the `.htaccess` and the maintenance page are generated + - **and** the `.htaccess` proxies to the port from the root configuration + +##### Verified by + +- [InitCommandTest: `--systemd reads the configuration from the repository root, not the current directory`](../../src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt) + +## The Solution + +The catch-all now prints the exception message before falling back: + +``` +Warning: the effective configuration could not be loaded () + continuing with default server settings — check the generated unit and host files +``` + +The configuration is read three times while the systemd files are written (`memoryMax`, `tasksMax`, `publicBaseUrl`), which would repeat the warning three times. +It is therefore loaded once per run and cached in the command, and the cache is reset at the top of `run()` so a reused instance — the command is a Spring singleton — re-reads. + +The repository root is passed down into the two accessors instead of `Paths.get(".")`. +This matches every other caller of `ConfigLoader.load` in the code base, all of which pass an explicit working directory; +`InitCommand` was the only one relying on the process's current directory. + +Both fixes are the same failure in two forms — the command answered from a configuration it never actually read — which is why they are in one PR. + +## Additional Changes + +- None. -- 2.39.5