Pin the trigger of a follow-up build to the host (PR#23, point 2)
A branch's committed config loses afterSuccessOf from every trigger, and where the host defines the same build as a follow-up, the branch's whole trigger block: a follow-up is the host's way to hand deployment targets and credentials to a green commit, so a branch may say what its deployment does, never that or where it happens — otherwise it could widen the host's selector to include itself. Both cases are logged naming the branch, which the watcher now passes along. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
06eafc8010
commit
ab522bc68d
@@ -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<String, Any?>,
|
||||
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<String, Any?>): Map<String, Any?> {
|
||||
private fun stripPinned(
|
||||
branchLayer: Map<String, Any?>,
|
||||
primary: Map<String, Any?>,
|
||||
branch: String?,
|
||||
): Map<String, Any?> {
|
||||
if (branchLayer.isEmpty()) {
|
||||
return branchLayer
|
||||
}
|
||||
@@ -206,9 +216,55 @@ class ConfigLoader(
|
||||
val entries = result[section] as? Map<String, Any?> ?: continue
|
||||
result[section] = entries.mapValues { (_, value) -> stripPinnedSettings(value) }
|
||||
}
|
||||
(result["builds"] as? Map<String, Any?>)?.let { builds ->
|
||||
val hostBuilds = primary["builds"] as? Map<String, Any?> ?: 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<String, Any?> ?: return value
|
||||
val trigger = definition["trigger"] as? Map<String, Any?> ?: 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<String, Any?> ?: return value
|
||||
|
||||
@@ -324,6 +324,7 @@ class Watcher(
|
||||
.loadWithBranchLayer(
|
||||
workingDir,
|
||||
ConfigFiles.readCommitted { gitService.showFileAtCommit(commit, it, workingDir) },
|
||||
branch,
|
||||
).effectiveBuildDefinitions()
|
||||
} catch (e: Exception) {
|
||||
log.warn(
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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")),
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user