Add trigger.afterSuccessOf, the follow-up build's predecessor (PR#23, point 1)
A build definition may name the build it follows. The key lives in the trigger block, so it is never inherited and refused when written flat; a definition with only a predecessor counts as triggered. An unknown predecessor refuses the start of the primary configuration — a deployment that silently never runs is the failure the flat-key check exists to prevent — while a branch layer that dropped the name only loses its follow-up with a warning, and an instance fragment is not checked alone since its predecessor may live in the project config. A cycle is refused wherever it appears. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
e4b935c684
commit
06eafc8010
@@ -89,8 +89,9 @@ data class BuildDefinition(
|
|||||||
* When a build runs and for which branches — the `trigger` block of a build definition,
|
* When a build runs and for which branches — the `trigger` block of a build definition,
|
||||||
* and the one part of it that is never inherited from `builds.default`.
|
* and the one part of it that is never inherited from `builds.default`.
|
||||||
*
|
*
|
||||||
* A definition with neither [onPush] nor [atTimes] never triggers automatically; that is
|
* A definition with neither [onPush] nor [atTimes] nor [afterSuccessOf] never triggers
|
||||||
* how `builds.default` is written when it is meant as a settings base only.
|
* automatically; that is how `builds.default` is written when it is meant as a settings
|
||||||
|
* base only.
|
||||||
*/
|
*/
|
||||||
data class TriggerConfig(
|
data class TriggerConfig(
|
||||||
/** Build every new commit of the selected branches. */
|
/** Build every new commit of the selected branches. */
|
||||||
@@ -112,7 +113,19 @@ data class TriggerConfig(
|
|||||||
* empty applies no age filter. Combines with [branches] as an intersection.
|
* empty applies no age filter. Combines with [branches] as an intersection.
|
||||||
*/
|
*/
|
||||||
val activeWithin: String = "",
|
val activeWithin: String = "",
|
||||||
|
/**
|
||||||
|
* Name of another definition of this configuration — the *predecessor*: this build
|
||||||
|
* runs on the predecessor's branch at the predecessor's commit whenever a run of it
|
||||||
|
* ends with `SUCCESS`, whatever started that run (PR#23). Empty means none. Sits in
|
||||||
|
* the trigger block because it says *when* this build runs, so it is never inherited;
|
||||||
|
* and it is host-pinned, because a follow-up build is the host's way to hand
|
||||||
|
* real-world effects — deployment targets, credentials — to a green commit.
|
||||||
|
*/
|
||||||
|
val afterSuccessOf: String = "",
|
||||||
) {
|
) {
|
||||||
|
/** True when this build follows another one, see [afterSuccessOf]. */
|
||||||
|
fun isFollowUp(): Boolean = afterSuccessOf.isNotBlank()
|
||||||
|
|
||||||
/** True when [branch] matches the [branches] patterns (or none are configured) and none excludes it. */
|
/** True when [branch] matches the [branches] patterns (or none are configured) and none excludes it. */
|
||||||
fun selectsByName(branch: String): Boolean {
|
fun selectsByName(branch: String): Boolean {
|
||||||
val (excluding, including) = branches.partition { it.startsWith(EXCLUDE_PREFIX) }
|
val (excluding, including) = branches.partition { it.startsWith(EXCLUDE_PREFIX) }
|
||||||
|
|||||||
@@ -122,15 +122,33 @@ class ConfigLoader(
|
|||||||
// must never stop the server or hold up the branches that are fine
|
// must never stop the server or hold up the branches that are fine
|
||||||
checkVersion(branchLayer, "the committed .werkator.yml of this branch", BRANCH_HINT)
|
checkVersion(branchLayer, "the committed .werkator.yml of this branch", BRANCH_HINT)
|
||||||
checkTriggerBlocks(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)))
|
return toConfig(deepMerge(loadRaw(workingDir), stripPinned(branchLayer)), MissingPredecessor.WARN)
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun toConfig(raw: Map<String, Any?>): WerkatorConfig {
|
/**
|
||||||
|
* What a follow-up whose predecessor no definition has means for this load, see
|
||||||
|
* [checkFollowUps].
|
||||||
|
*/
|
||||||
|
private enum class MissingPredecessor {
|
||||||
|
/** The primary configuration: a deployment that could never fire refuses the start. */
|
||||||
|
REFUSE,
|
||||||
|
|
||||||
|
/** A branch layer on top: the branch renamed or dropped the build the host's trigger names, and only loses its follow-up. */
|
||||||
|
WARN,
|
||||||
|
|
||||||
|
/** A fragment checked on its own: the predecessor may well live in the project config it is merged with later. */
|
||||||
|
SKIP,
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun toConfig(
|
||||||
|
raw: Map<String, Any?>,
|
||||||
|
missingPredecessor: MissingPredecessor = MissingPredecessor.REFUSE,
|
||||||
|
): WerkatorConfig {
|
||||||
val config =
|
val config =
|
||||||
if (raw.isEmpty()) {
|
if (raw.isEmpty()) {
|
||||||
WerkatorConfig()
|
WerkatorConfig()
|
||||||
} else {
|
} else {
|
||||||
yaml.convertValue(resolveBuildSections(dropNonDefinitionBuilds(raw)), WerkatorConfig::class.java)
|
yaml.convertValue(resolveBuildSections(dropNonDefinitionBuilds(raw), missingPredecessor), WerkatorConfig::class.java)
|
||||||
}
|
}
|
||||||
return defaultPublicBaseUrl(config)
|
return defaultPublicBaseUrl(config)
|
||||||
}
|
}
|
||||||
@@ -225,12 +243,16 @@ class ConfigLoader(
|
|||||||
* an empty docker policy and run natively on the host, which is exactly the escape
|
* an empty docker policy and run natively on the host, which is exactly the escape
|
||||||
* the pinned keys exist to prevent.
|
* the pinned keys exist to prevent.
|
||||||
*/
|
*/
|
||||||
private fun resolveBuildSections(raw: Map<String, Any?>): Map<String, Any?> {
|
private fun resolveBuildSections(
|
||||||
|
raw: Map<String, Any?>,
|
||||||
|
missingPredecessor: MissingPredecessor,
|
||||||
|
): Map<String, Any?> {
|
||||||
@Suppress("UNCHECKED_CAST")
|
@Suppress("UNCHECKED_CAST")
|
||||||
val definitions = raw["builds"] as? Map<String, Any?> ?: emptyMap()
|
val definitions = raw["builds"] as? Map<String, Any?> ?: emptyMap()
|
||||||
if (definitions.isEmpty()) {
|
if (definitions.isEmpty()) {
|
||||||
return mergeBranchDefaults(raw)
|
return mergeBranchDefaults(raw)
|
||||||
}
|
}
|
||||||
|
checkFollowUps(definitions, missingPredecessor)
|
||||||
if (raw.containsKey("branches") && warnedSections.add(LEGACY_BRANCHES_WARNING)) {
|
if (raw.containsKey("branches") && warnedSections.add(LEGACY_BRANCHES_WARNING)) {
|
||||||
log.warn(
|
log.warn(
|
||||||
"ignoring the branches section: this configuration defines builds, and a build definition " +
|
"ignoring the branches section: this configuration defines builds, and a build definition " +
|
||||||
@@ -252,13 +274,59 @@ class ConfigLoader(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
if (warnedSections.add(NO_TRIGGER_WARNING)) {
|
if (warnedSections.add(NO_TRIGGER_WARNING)) {
|
||||||
log.warn("no build defines onPush or atTimes; the watcher will never start a build on its own")
|
log.warn("no build defines onPush, atTimes, or afterSuccessOf; the watcher will never start a build on its own")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun isTriggered(definition: Any?): Boolean {
|
private fun isTriggered(definition: Any?): Boolean {
|
||||||
val trigger = (definition as? Map<*, *>)?.get("trigger") as? Map<*, *> ?: return false
|
val trigger = (definition as? Map<*, *>)?.get("trigger") as? Map<*, *> ?: return false
|
||||||
return trigger["onPush"] == true || (trigger["atTimes"] as? List<*>)?.isNotEmpty() == true
|
return trigger["onPush"] == true ||
|
||||||
|
(trigger["atTimes"] as? List<*>)?.isNotEmpty() == true ||
|
||||||
|
predecessorOf(definition) != null
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun predecessorOf(definition: Any?): String? {
|
||||||
|
val trigger = (definition as? Map<*, *>)?.get("trigger") as? Map<*, *> ?: return null
|
||||||
|
return (trigger["afterSuccessOf"] as? String)?.takeIf { it.isNotBlank() }
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A follow-up build that could never fire must not exist, for the same reason a flat
|
||||||
|
* trigger key is refused: a deployment that silently never runs is worse than a
|
||||||
|
* configuration that refuses to load (PR#23). Refused are a predecessor no definition
|
||||||
|
* has, and a cycle of follow-ups (a build following itself included) — a cycle is a
|
||||||
|
* configuration error whoever wrote it, while a missing predecessor depends on what
|
||||||
|
* is being loaded ([MissingPredecessor]).
|
||||||
|
*/
|
||||||
|
private fun checkFollowUps(
|
||||||
|
definitions: Map<String, Any?>,
|
||||||
|
missingPredecessor: MissingPredecessor,
|
||||||
|
) {
|
||||||
|
val predecessors = definitions.mapValues { (_, definition) -> predecessorOf(definition) }
|
||||||
|
val effective = definitions.keys + BuildDefinition.DEFAULT
|
||||||
|
for ((name, predecessor) in predecessors) {
|
||||||
|
if (predecessor == null || predecessor in effective) continue
|
||||||
|
val message = "builds.$name follows '$predecessor' (trigger.afterSuccessOf), but no build of that name is defined"
|
||||||
|
when (missingPredecessor) {
|
||||||
|
MissingPredecessor.REFUSE -> throw ConfigFormatException("$message. Name an existing build, or remove the follow-up.")
|
||||||
|
MissingPredecessor.WARN -> log.warn("$message on this branch; the follow-up will not run for it")
|
||||||
|
MissingPredecessor.SKIP -> {}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for (start in predecessors.keys) {
|
||||||
|
val path = mutableListOf(start)
|
||||||
|
var current = predecessors[start]
|
||||||
|
while (current != null && current !in path) {
|
||||||
|
path += current
|
||||||
|
current = predecessors[current]
|
||||||
|
}
|
||||||
|
if (current == start) {
|
||||||
|
throw ConfigFormatException(
|
||||||
|
"builds.$start follows itself through trigger.afterSuccessOf (${path.joinToString(" -> ")} -> $start); " +
|
||||||
|
"a follow-up build cannot wait for its own success.",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -426,7 +494,10 @@ class ConfigLoader(
|
|||||||
checkVersion(raw, fragment.toString(), ROLLBACK_HINT)
|
checkVersion(raw, fragment.toString(), ROLLBACK_HINT)
|
||||||
checkTriggerBlocks(raw, fragment.toString(), ROLLBACK_HINT)
|
checkTriggerBlocks(raw, fragment.toString(), ROLLBACK_HINT)
|
||||||
try {
|
try {
|
||||||
strictYaml.convertValue(resolveBuildSections(dropNonDefinitionBuilds(raw)), WerkatorConfig::class.java)
|
strictYaml.convertValue(
|
||||||
|
resolveBuildSections(dropNonDefinitionBuilds(raw), MissingPredecessor.SKIP),
|
||||||
|
WerkatorConfig::class.java,
|
||||||
|
)
|
||||||
} catch (e: IllegalArgumentException) {
|
} catch (e: IllegalArgumentException) {
|
||||||
throw IllegalArgumentException(
|
throw IllegalArgumentException(
|
||||||
"instance fragment $fragment does not match the configuration schema: ${e.message}",
|
"instance fragment $fragment does not match the configuration schema: ${e.message}",
|
||||||
@@ -610,7 +681,7 @@ class ConfigLoader(
|
|||||||
private val TRIGGER_KEYS = setOf("trigger")
|
private val TRIGGER_KEYS = setOf("trigger")
|
||||||
|
|
||||||
/** The keys that moved into [TRIGGER_KEYS]; still writing them flat is refused, not ignored. */
|
/** The keys that moved into [TRIGGER_KEYS]; still writing them flat is refused, not ignored. */
|
||||||
private val FLAT_TRIGGER_KEYS = setOf("onPush", "atTimes", "branches", "activeWithin")
|
private val FLAT_TRIGGER_KEYS = setOf("onPush", "atTimes", "branches", "activeWithin", "afterSuccessOf")
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Top-level sections owned by the instance once a home config exists (ADR 0009):
|
* Top-level sections owned by the instance once a home config exists (ADR 0009):
|
||||||
|
|||||||
@@ -739,6 +739,117 @@ class ConfigLoaderTest : FunSpec() {
|
|||||||
.shouldBeTrue()
|
.shouldBeTrue()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
test("afterSuccessOf must name an existing definition and must not form a cycle") {
|
||||||
|
val dir = Files.createTempDirectory("werkator-test")
|
||||||
|
val project = dir.resolve(".werkator.yml").toFile()
|
||||||
|
project.writeText(
|
||||||
|
"""
|
||||||
|
builds:
|
||||||
|
default:
|
||||||
|
trigger:
|
||||||
|
onPush: true
|
||||||
|
deploy:
|
||||||
|
trigger:
|
||||||
|
afterSuccessOf: default
|
||||||
|
branches: ["main"]
|
||||||
|
buildCommand: scripts/deploy.sh
|
||||||
|
""".trimIndent(),
|
||||||
|
)
|
||||||
|
// a follow-up with nothing but its predecessor is a triggered build, and it binds
|
||||||
|
val deploy = loader.load(dir).buildDefinitions.getValue("deploy")
|
||||||
|
deploy.trigger.afterSuccessOf shouldBe "default"
|
||||||
|
deploy.trigger.isFollowUp().shouldBeTrue()
|
||||||
|
deploy.trigger.onPush.shouldBeFalse()
|
||||||
|
|
||||||
|
project.writeText(
|
||||||
|
"""
|
||||||
|
builds:
|
||||||
|
deploy:
|
||||||
|
trigger:
|
||||||
|
afterSuccessOf: frontend
|
||||||
|
""".trimIndent(),
|
||||||
|
)
|
||||||
|
shouldThrow<ConfigFormatException> { loader.load(dir) }.message.let {
|
||||||
|
it.shouldContain("builds.deploy")
|
||||||
|
it.shouldContain("frontend")
|
||||||
|
}
|
||||||
|
|
||||||
|
project.writeText(
|
||||||
|
"""
|
||||||
|
builds:
|
||||||
|
a:
|
||||||
|
trigger:
|
||||||
|
afterSuccessOf: b
|
||||||
|
b:
|
||||||
|
trigger:
|
||||||
|
afterSuccessOf: a
|
||||||
|
""".trimIndent(),
|
||||||
|
)
|
||||||
|
shouldThrow<ConfigFormatException> { loader.load(dir) }.message.shouldContain("a -> b -> a")
|
||||||
|
|
||||||
|
project.writeText(
|
||||||
|
"""
|
||||||
|
builds:
|
||||||
|
a:
|
||||||
|
trigger:
|
||||||
|
afterSuccessOf: a
|
||||||
|
""".trimIndent(),
|
||||||
|
)
|
||||||
|
shouldThrow<ConfigFormatException> { loader.load(dir) }.message.shouldContain("builds.a follows itself")
|
||||||
|
}
|
||||||
|
|
||||||
|
test("a branch whose layer lacks the predecessor loses only its follow-up") {
|
||||||
|
val dir = Files.createTempDirectory("werkator-test")
|
||||||
|
dir.resolve(".werkator.yml").toFile().writeText(
|
||||||
|
"""
|
||||||
|
builds:
|
||||||
|
frontend:
|
||||||
|
trigger:
|
||||||
|
onPush: true
|
||||||
|
deploy:
|
||||||
|
trigger:
|
||||||
|
afterSuccessOf: frontend
|
||||||
|
""".trimIndent(),
|
||||||
|
)
|
||||||
|
// the branch renamed the predecessor: a warning, not a failed load — the branch's
|
||||||
|
// own builds must keep running, its follow-up simply never fires for it
|
||||||
|
val config =
|
||||||
|
loader.loadWithBranchLayer(
|
||||||
|
dir,
|
||||||
|
"""
|
||||||
|
builds:
|
||||||
|
frontend: null
|
||||||
|
ui:
|
||||||
|
trigger:
|
||||||
|
onPush: true
|
||||||
|
""".trimIndent(),
|
||||||
|
)
|
||||||
|
config.buildDefinitions
|
||||||
|
.getValue("ui")
|
||||||
|
.trigger.onPush
|
||||||
|
.shouldBeTrue()
|
||||||
|
config.buildDefinitions
|
||||||
|
.getValue("deploy")
|
||||||
|
.trigger.afterSuccessOf shouldBe "frontend"
|
||||||
|
}
|
||||||
|
|
||||||
|
test("afterSuccessOf written flat is refused like every trigger key") {
|
||||||
|
val dir = Files.createTempDirectory("werkator-test")
|
||||||
|
dir.resolve(".werkator.yml").toFile().writeText(
|
||||||
|
"""
|
||||||
|
builds:
|
||||||
|
default:
|
||||||
|
trigger:
|
||||||
|
onPush: true
|
||||||
|
deploy:
|
||||||
|
afterSuccessOf: default
|
||||||
|
""".trimIndent(),
|
||||||
|
)
|
||||||
|
val thrown = shouldThrow<ConfigFormatException> { loader.load(dir) }
|
||||||
|
thrown.message.shouldContain("builds.deploy: afterSuccessOf")
|
||||||
|
thrown.message.shouldContain("trigger:")
|
||||||
|
}
|
||||||
|
|
||||||
test("builds.default is the base of every other build, but never its trigger") {
|
test("builds.default is the base of every other build, but never its trigger") {
|
||||||
val dir = Files.createTempDirectory("werkator-test")
|
val dir = Files.createTempDirectory("werkator-test")
|
||||||
dir.resolve(".werkator.yml").toFile().writeText(
|
dir.resolve(".werkator.yml").toFile().writeText(
|
||||||
|
|||||||
Reference in New Issue
Block a user