init reads its configuration from the repository root, and says so when it cannot (#22)
`init --systemd` generated the systemd unit, the .htaccess and the maintenance page from a configuration it never actually read reliably: the load swallowed every exception and fell back to a default `ServerConfig`, and it resolved the layers from the process's current directory instead of the git top level every other file of the command goes through. A broken `.werkator.yml` was therefore indistinguishable from an unconfigured `publicBaseUrl` — the host integration was skipped without a word — and running `init` from a subdirectory read a foreign configuration or none, which also defeated `--apply`'s promise that the fragment reaches the generated unit. The fallback stays, but the exception message is printed, once per run, and the repository root is passed down like every other caller of `ConfigLoader.load`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
092183ca30
commit
cd6f915231
@@ -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 (<message>)
|
||||||
|
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.
|
||||||
@@ -48,6 +48,7 @@ class InitCommand(
|
|||||||
internal var javaExecutableResolver: () -> Path = { Paths.get(System.getProperty("java.home"), "bin", "java") }
|
internal var javaExecutableResolver: () -> Path = { Paths.get(System.getProperty("java.home"), "bin", "java") }
|
||||||
|
|
||||||
override fun run() {
|
override fun run() {
|
||||||
|
cachedServerConfig = null
|
||||||
val normalizedWorkingDir = workingDir.toAbsolutePath().normalize()
|
val normalizedWorkingDir = workingDir.toAbsolutePath().normalize()
|
||||||
val root =
|
val root =
|
||||||
try {
|
try {
|
||||||
@@ -296,14 +297,31 @@ class InitCommand(
|
|||||||
* already loadable (re-running `init --systemd` on an installed instance); during
|
* already loadable (re-running `init --systemd` on an installed instance); during
|
||||||
* the very first bootstrap they stay unset and the defaults (no directives) apply.
|
* 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 =
|
/** 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 {
|
try {
|
||||||
configLoader.load(Paths.get(".")).server
|
configLoader.load(root).server
|
||||||
} catch (_: Exception) {
|
} 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
|
de.hoennig.werkator.config
|
||||||
.ServerConfig()
|
.ServerConfig()
|
||||||
|
}.also { cachedServerConfig = it }
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun createSystemdFiles(
|
private fun createSystemdFiles(
|
||||||
@@ -327,8 +345,8 @@ class InitCommand(
|
|||||||
javaExecutable = javaExecutableResolver(),
|
javaExecutable = javaExecutableResolver(),
|
||||||
jarPath = jarPath,
|
jarPath = jarPath,
|
||||||
envFile = envFile,
|
envFile = envFile,
|
||||||
memoryMax = loadedSystemdConfig().memoryMax,
|
memoryMax = loadedSystemdConfig(root).memoryMax,
|
||||||
tasksMax = loadedSystemdConfig().tasksMax,
|
tasksMax = loadedSystemdConfig(root).tasksMax,
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
println("created ${unitFile.toFile().relativeTo(normalizedWorkingDir.toFile())}")
|
println("created ${unitFile.toFile().relativeTo(normalizedWorkingDir.toFile())}")
|
||||||
@@ -351,7 +369,7 @@ class InitCommand(
|
|||||||
|
|
||||||
// generated host integration like the units: only meaningful behind a web
|
// generated host integration like the units: only meaningful behind a web
|
||||||
// frontend, so it needs a public base URL; unused elsewhere and harmless
|
// frontend, so it needs a public base URL; unused elsewhere and harmless
|
||||||
val server = loadedServerConfig()
|
val server = loadedServerConfig(root)
|
||||||
if (server.publicBaseUrl.isNotBlank()) {
|
if (server.publicBaseUrl.isNotBlank()) {
|
||||||
val htaccessFile = werkatorDir.resolve(SystemdServiceFiles.HTACCESS_NAME)
|
val htaccessFile = werkatorDir.resolve(SystemdServiceFiles.HTACCESS_NAME)
|
||||||
htaccessFile.toFile().writeText(SystemdServiceFiles.htaccessContent(server.port))
|
htaccessFile.toFile().writeText(SystemdServiceFiles.htaccessContent(server.port))
|
||||||
|
|||||||
@@ -252,5 +252,50 @@ class InitCommandTest : FunSpec() {
|
|||||||
// This should not throw IllegalArgumentException
|
// This should not throw IllegalArgumentException
|
||||||
initCommand.run()
|
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<de.hoennig.werkator.config.ConfigLoader>()
|
||||||
|
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()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user