diff --git a/src/main/kotlin/de/hoennig/werkator/config/ConfigLoader.kt b/src/main/kotlin/de/hoennig/werkator/config/ConfigLoader.kt index c6c4027..294ee1f 100644 --- a/src/main/kotlin/de/hoennig/werkator/config/ConfigLoader.kt +++ b/src/main/kotlin/de/hoennig/werkator/config/ConfigLoader.kt @@ -90,7 +90,8 @@ class ConfigLoader( fun loadForWorktree( workingDir: Path, worktreeDir: Path, - ): WerkatorConfig = withBranchLayer(workingDir, loadFile(worktreeDir.resolve(ConfigFiles.firstExisting(worktreeDir)).toFile())) + branch: String? = null, + ): WerkatorConfig = withBranchLayer(workingDir, loadFile(worktreeDir.resolve(ConfigFiles.firstExisting(worktreeDir)).toFile()), branch) /** * The primary/`.git` config with the committed `.werkator.yml` of one branch @@ -107,22 +108,27 @@ class ConfigLoader( * (`requirePullRequest`, which decides whether the branch is built at all). * They are stripped from the branch layer before merging, so a branch can neither * escape its container, nor bypass its own pull-request gate, nor raise the global - * concurrency, nor reach the credentials. + * concurrency, nor reach the credentials. The trigger of a follow-up build is pinned + * the same way (PR#23): a branch may say what its deployment does, never that — or + * where — it happens. [branch] only names the branch in the warnings. */ fun loadWithBranchLayer( workingDir: Path, branchConfigYaml: String?, - ): WerkatorConfig = withBranchLayer(workingDir, parseYaml(branchConfigYaml)) + branch: String? = null, + ): WerkatorConfig = withBranchLayer(workingDir, parseYaml(branchConfigYaml), branch) private fun withBranchLayer( workingDir: Path, branchLayer: Map, + branch: String?, ): WerkatorConfig { // scoped to this branch: an incompatible branch config fails its own builds and // must never stop the server or hold up the branches that are fine checkVersion(branchLayer, "the committed .werkator.yml of this branch", BRANCH_HINT) checkTriggerBlocks(branchLayer, "the committed .werkator.yml of this branch", BRANCH_HINT) - return toConfig(deepMerge(loadRaw(workingDir), stripPinned(branchLayer)), MissingPredecessor.WARN) + val primary = loadRaw(workingDir) + return toConfig(deepMerge(primary, stripPinned(branchLayer, primary, branch)), MissingPredecessor.WARN) } /** @@ -196,7 +202,11 @@ class ConfigLoader( * second as soon as the committed configuration carries them. */ @Suppress("UNCHECKED_CAST") - private fun stripPinned(branchLayer: Map): Map { + private fun stripPinned( + branchLayer: Map, + primary: Map, + branch: String?, + ): Map { if (branchLayer.isEmpty()) { return branchLayer } @@ -206,9 +216,55 @@ class ConfigLoader( val entries = result[section] as? Map ?: continue result[section] = entries.mapValues { (_, value) -> stripPinnedSettings(value) } } + (result["builds"] as? Map)?.let { builds -> + val hostBuilds = primary["builds"] as? Map ?: emptyMap() + result["builds"] = builds.mapValues { (name, value) -> stripPinnedTrigger(name, value, hostBuilds[name], branch) } + } return result } + /** + * The follow-up part of the pinning (PR#23): a branch's definition loses its + * `afterSuccessOf`, and where the host's definition of the same name is a follow-up, + * the branch's whole `trigger` block — otherwise a branch could widen the host's + * selector to include itself, and deploy itself with the host's credentials. Said + * out loud, because a trigger the branch wrote and does not see in effect is a + * question it would otherwise ask the log in vain. + */ + @Suppress("UNCHECKED_CAST") + private fun stripPinnedTrigger( + name: String, + value: Any?, + hostDefinition: Any?, + branch: String?, + ): Any? { + val definition = value as? Map ?: return value + val trigger = definition["trigger"] as? Map ?: return value + val where = branch?.let { "branch '$it'" } ?: "this branch" + if (predecessorOf(hostDefinition) != null) { + log.warn( + "ignoring the trigger block of builds.{} in the committed {} of {}: the host defines that build as a follow-up, " + + "and when and where a follow-up runs is the host's decision alone", + name, + ConfigFiles.COMMITTED, + where, + ) + return definition - "trigger" + } + if (predecessorOf(definition) == null) { + return value + } + log.warn( + "ignoring builds.{}.trigger.afterSuccessOf in the committed {} of {}: a branch cannot make a build follow another, " + + "only the host can", + name, + ConfigFiles.COMMITTED, + where, + ) + val stripped = trigger - "afterSuccessOf" + return if (stripped.isEmpty()) definition - "trigger" else definition + ("trigger" to stripped) + } + @Suppress("UNCHECKED_CAST") private fun stripPinnedSettings(value: Any?): Any? { val entry = value as? Map ?: return value diff --git a/src/main/kotlin/de/hoennig/werkator/watcher/Watcher.kt b/src/main/kotlin/de/hoennig/werkator/watcher/Watcher.kt index bdb627d..7f2566b 100644 --- a/src/main/kotlin/de/hoennig/werkator/watcher/Watcher.kt +++ b/src/main/kotlin/de/hoennig/werkator/watcher/Watcher.kt @@ -324,6 +324,7 @@ class Watcher( .loadWithBranchLayer( workingDir, ConfigFiles.readCommitted { gitService.showFileAtCommit(commit, it, workingDir) }, + branch, ).effectiveBuildDefinitions() } catch (e: Exception) { log.warn( diff --git a/src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt b/src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt index b8ab53b..5c0d63e 100644 --- a/src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt @@ -850,6 +850,79 @@ class ConfigLoaderTest : FunSpec() { thrown.message.shouldContain("trigger:") } + test("a follow-up trigger is pinned to the host, a branch cannot add or widen one") { + val dir = Files.createTempDirectory("werkator-test") + dir.resolve(".werkator.yml").toFile().writeText( + """ + builds: + frontend: + trigger: + onPush: true + deploy: + trigger: + afterSuccessOf: frontend + branches: ["main"] + """.trimIndent(), + ) + val config = + loader.loadWithBranchLayer( + dir, + """ + builds: + deploy: + trigger: + afterSuccessOf: frontend + branches: ["*"] + nightly: + trigger: + atTimes: ["01:00"] + afterSuccessOf: frontend + """.trimIndent(), + "feature/x", + ) + // the host's trigger block of the follow-up is used unchanged + val deploy = config.buildDefinitions.getValue("deploy").trigger + deploy.afterSuccessOf shouldBe "frontend" + deploy.branches shouldBe listOf("main") + deploy.selectsByName("feature/x").shouldBeFalse() + // and a branch cannot make any build of its own a follow-up + val nightly = config.buildDefinitions.getValue("nightly").trigger + nightly.afterSuccessOf shouldBe "" + nightly.atTimes shouldBe listOf("01:00") + } + + test("a branch supplies the command of a host-triggered follow-up") { + val dir = Files.createTempDirectory("werkator-test") + Files.createDirectories(dir.resolve(".git/werkator")) + dir.resolve(".git/werkator/.werkator.yml").toFile().writeText( + """ + builds: + deploy: + trigger: + afterSuccessOf: default + branches: ["main"] + werkdock: + env: + DEPLOY_TARGET: host:/srv/www + """.trimIndent(), + ) + val worktree = Files.createTempDirectory("werkator-test-worktree") + worktree.resolve(".werkator.yml").toFile().writeText( + """ + builds: + deploy: + cleanCommand: "" + buildCommand: scripts/deploy-prod.sh -y "${'$'}DEPLOY_TARGET" + """.trimIndent(), + ) + + val settings = loader.loadForWorktree(dir, worktree, "main").buildSettings("main", "deploy") + + settings.buildCommand shouldBe "scripts/deploy-prod.sh -y \"${'$'}DEPLOY_TARGET\"" + settings.cleanCommand shouldBe "" + settings.werkdock.env shouldBe mapOf("DEPLOY_TARGET" to "host:/srv/www") + } + test("builds.default is the base of every other build, but never its trigger") { val dir = Files.createTempDirectory("werkator-test") dir.resolve(".werkator.yml").toFile().writeText( diff --git a/src/test/kotlin/de/hoennig/werkator/server/PermanentBranchRoutesTest.kt b/src/test/kotlin/de/hoennig/werkator/server/PermanentBranchRoutesTest.kt index aa47ab8..4be9aed 100644 --- a/src/test/kotlin/de/hoennig/werkator/server/PermanentBranchRoutesTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/server/PermanentBranchRoutesTest.kt @@ -113,7 +113,7 @@ class PermanentBranchRoutesTest : FunSpec() { every { registry.byName(any()) } returns null every { registry.byName("test") } returns repo every { configLoader.load(any()) } returns WerkatorConfig() - every { configLoader.loadWithBranchLayer(any(), anyNullable()) } returns WerkatorConfig() + every { configLoader.loadWithBranchLayer(any(), anyNullable(), anyNullable()) } returns WerkatorConfig() every { gitService.showFileAtCommit(any(), any(), any()) } returns null every { controlTokens.token() } returns "test-token" every { branchListing.branches(any()) } returns emptyList() diff --git a/src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt b/src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt index 2b6c83e..cfd9005 100644 --- a/src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt @@ -142,7 +142,7 @@ class UiControllerTest : FunSpec() { server = ServerConfig(impressumUrl = "https://example.org/imprint"), gitea = GiteaConfig(baseUrl = "https://git.example.org", owner = "acme", repo = "widget"), ) - every { configLoader.loadWithBranchLayer(any(), anyNullable()) } returns WerkatorConfig() + every { configLoader.loadWithBranchLayer(any(), anyNullable(), anyNullable()) } returns WerkatorConfig() every { gitService.showFileAtCommit(any(), any(), any()) } returns null every { controlTokens.token() } returns "test-token" every { repository.latestGreenFor(any()) } returns null @@ -385,7 +385,7 @@ class UiControllerTest : FunSpec() { ) every { repository.history() } returns listOf(pitestResult) every { artifactStore.artifactDir("main-pitest-key") } returns null - every { configLoader.loadWithBranchLayer(any(), anyNullable()) } returns + every { configLoader.loadWithBranchLayer(any(), anyNullable(), anyNullable()) } returns WerkatorConfig( branches = mapOf("default" to BranchConfig(buildCommand = "./gradlew quick-check")), buildDefinitions = mapOf("pitest" to BuildDefinition(buildCommand = "./gradlew pitestFull")), diff --git a/src/test/kotlin/de/hoennig/werkator/watcher/WatcherTest.kt b/src/test/kotlin/de/hoennig/werkator/watcher/WatcherTest.kt index 218d798..bb2b4c7 100644 --- a/src/test/kotlin/de/hoennig/werkator/watcher/WatcherTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/watcher/WatcherTest.kt @@ -85,7 +85,7 @@ class WatcherTest : FunSpec() { every { gitService.originBranchCommitTimes(any()) } returns emptyMap() every { gitService.originBranchHeads(any()) } returns emptyMap() every { gitService.showFileAtCommit(any(), any(), any()) } returns null - every { configLoader.loadWithBranchLayer(any(), anyNullable()) } returns config + every { configLoader.loadWithBranchLayer(any(), anyNullable(), anyNullable()) } returns config every { gitService.pullRequestHeads(any()) } returns emptySet() every { gitService.worktreePrune(any()) } returns Unit every { gitService.fastForwardLocalBranches(any()) } returns emptyList() @@ -526,7 +526,7 @@ class WatcherTest : FunSpec() { every { harness.gitService.originBranchHeads(any()) } returns mapOf("main" to "commit-main", "experiment" to "commit-exp") every { harness.gitService.showFileAtCommit("commit-exp", Watcher.CONFIG_FILE, any()) } returns "branch-yaml" - every { harness.configLoader.loadWithBranchLayer(any(), "branch-yaml") } returns branchLayer + every { harness.configLoader.loadWithBranchLayer(any(), "branch-yaml", anyNullable()) } returns branchLayer every { harness.gitService.originHeadCommit("experiment", any()) } returns "commit-exp" every { harness.gitService.originHeadCommit("main", any()) } returns "commit-main" @@ -549,7 +549,7 @@ class WatcherTest : FunSpec() { every { harness.gitService.originBranches(any()) } returns listOf("experiment") every { harness.gitService.originBranchHeads(any()) } returns mapOf("experiment" to "commit-exp") every { harness.gitService.showFileAtCommit("commit-exp", ".gittally.yml", any()) } returns "branch-yaml" - every { harness.configLoader.loadWithBranchLayer(any(), "branch-yaml") } returns branchLayer + every { harness.configLoader.loadWithBranchLayer(any(), "branch-yaml", anyNullable()) } returns branchLayer every { harness.gitService.originHeadCommit("experiment", any()) } returns "commit-exp" harness.watcher.poll(harness.repo) @@ -568,7 +568,7 @@ class WatcherTest : FunSpec() { every { harness.gitService.originBranchHeads(any()) } returns mapOf("main" to "commit-main", "experiment" to "commit-exp") every { harness.gitService.showFileAtCommit("commit-exp", Watcher.CONFIG_FILE, any()) } returns "branch-yaml" - every { harness.configLoader.loadWithBranchLayer(any(), "branch-yaml") } returns branchLayer + every { harness.configLoader.loadWithBranchLayer(any(), "branch-yaml", anyNullable()) } returns branchLayer every { harness.gitService.originHeadCommit(any(), any()) } returns "commit-any" harness.watcher.poll(harness.repo) @@ -608,7 +608,7 @@ class WatcherTest : FunSpec() { buildDefinitions = mapOf("nightly" to BuildDefinition(trigger = TriggerConfig(atTimes = listOf("11:00")))), ) every { harness.configLoader.load(any()) } returns edited - every { harness.configLoader.loadWithBranchLayer(any(), anyNullable()) } returns edited + every { harness.configLoader.loadWithBranchLayer(any(), anyNullable(), anyNullable()) } returns edited harness.watcher.poll(harness.repo) @@ -622,7 +622,7 @@ class WatcherTest : FunSpec() { every { harness.gitService.hasNewCommits("main", any()) } returns true every { harness.gitService.originBranchHeads(any()) } returns mapOf("main" to "commit-main") every { harness.gitService.showFileAtCommit("commit-main", Watcher.CONFIG_FILE, any()) } returns "broken" - every { harness.configLoader.loadWithBranchLayer(any(), "broken") } throws + every { harness.configLoader.loadWithBranchLayer(any(), "broken", anyNullable()) } throws RuntimeException("mapping problem") every { harness.gitService.originHeadCommit("main", any()) } returns "commit-main"