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. diff --git a/src/main/kotlin/de/hoennig/werkator/commands/InitCommand.kt b/src/main/kotlin/de/hoennig/werkator/commands/InitCommand.kt index f7a8616..f3c86f7 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 { @@ -296,14 +297,31 @@ 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 - private fun loadedServerConfig(): de.hoennig.werkator.config.ServerConfig = - try { - configLoader.load(Paths.get(".")).server - } catch (_: Exception) { - de.hoennig.werkator.config - .ServerConfig() + /** 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(root: Path): de.hoennig.werkator.config.ServerConfig = + cachedServerConfig ?: run { + try { + 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") + de.hoennig.werkator.config + .ServerConfig() + }.also { cachedServerConfig = it } } private fun createSystemdFiles( @@ -327,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())}") @@ -351,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 14ae12d..a2f289f 100644 --- a/src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/commands/InitCommandTest.kt @@ -252,5 +252,50 @@ class InitCommandTest : FunSpec() { // This should not throw IllegalArgumentException 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() + 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() + } } }