A build definition says when it runs in a trigger block of its own
`onPush`, `atTimes`, `branches`, and `activeWithin` move into a nested `trigger`. The split is structural on purpose: the inheritance from `builds.default` now subtracts one key instead of a list of four, so a selector added to `TriggerConfig` later is non-inheritable by construction rather than because someone remembered to extend the list. A definition still writing those keys flat is refused by name, per file and scoped like the version check — the machine and project config abort the start, a branch's committed config fails only that branch. Ignoring them would leave the build with no trigger at all, which is a job that quietly stops running: the failure this refusal exists to prevent. Two more things a definition can now say: - A `!` prefix in `trigger.branches` excludes, and an exclusion wins whatever the order. `["*", "!master"]` gives one branch a build of its own without the default build running over it as well — until now the only way out of that double build was to drop the second definition's push trigger. - `statusContext` overrides the Gitea check this build reports as, empty keeping the repository-wide one. Two builds of a commit shared a context and overwrote each other's result, so a quick check beside a long build was not readable in Gitea. Pinned like `requirePullRequest`: a branch that could pick its context could take over the check a branch protection rule depends on. Fixed on the way: a branch whose builds all belong to named definitions rendered an empty row in the branches view, reading as "never built" directly beside its real builds. That row was unreachable before the exclusion patterns made such a branch possible.
This commit is contained in:
mhoennig
@@ -8,16 +8,82 @@ import java.time.Instant
|
||||
* top-level `builds` section next to the reserved execution key `maxConcurrent`
|
||||
* (split apart by [ConfigLoader]).
|
||||
*
|
||||
* A definition carries the complete description of one build. The `default` entry is
|
||||
* additionally the base every other definition inherits its settings from — but never
|
||||
* its trigger, see [SELECTOR_KEYS][ConfigLoader]. Unset values fall through to the
|
||||
* [BranchConfig] defaults.
|
||||
*
|
||||
* The `default` build records its results under the plain branch name; every other
|
||||
* build records under `<branch>@<name>` with its own history, retention pool, and
|
||||
* permanent latest-green link.
|
||||
* A definition describes one build completely, in two halves: [trigger] says when it
|
||||
* runs and for which branches, everything else says what it does. The `default` entry
|
||||
* is additionally the base every other definition inherits from — its settings, never
|
||||
* its [trigger]. That is why the halves are separated structurally instead of by a list
|
||||
* of key names: a selector added to [TriggerConfig] later is non-inheritable by
|
||||
* construction, not because someone remembered to extend a list.
|
||||
*/
|
||||
data class BuildDefinition(
|
||||
/** When and for which branches this build runs; never inherited from `builds.default`. */
|
||||
val trigger: TriggerConfig = TriggerConfig(),
|
||||
/** Overrides the build command; null inherits it. */
|
||||
val buildCommand: String? = null,
|
||||
/** Overrides the clean command; null inherits it. */
|
||||
val cleanCommand: String? = null,
|
||||
/** Overrides the artifact directories; null inherits them. */
|
||||
val artifactDirs: List<String>? = null,
|
||||
/** Overrides the stdout log file name; null inherits it. */
|
||||
val stdoutLog: String? = null,
|
||||
/** Overrides the stderr log file name; null inherits it. */
|
||||
val stderrLog: String? = null,
|
||||
/**
|
||||
* The watcher builds a selected branch only while its head commit matches a
|
||||
* pull-request head; null inherits. Pinned — a branch's own committed config can
|
||||
* never set it, or it would bypass its own gate.
|
||||
*/
|
||||
val requirePullRequest: Boolean? = null,
|
||||
/**
|
||||
* Gitea commit status context of this build; null uses `gitea.statusContext`.
|
||||
* Two builds of the same commit under the same context overwrite each other's
|
||||
* result, so a second build over a branch needs its own context to be readable.
|
||||
*/
|
||||
val statusContext: String? = null,
|
||||
/** Overrides of the docker settings; null inherits them. */
|
||||
val docker: DockerOverrides? = null,
|
||||
) {
|
||||
/** The settings this build runs with: [branchConfig] with this definition applied; unset values fall through. */
|
||||
fun applyTo(branchConfig: BranchConfig): BranchConfig =
|
||||
branchConfig.copy(
|
||||
buildCommand = buildCommand ?: branchConfig.buildCommand,
|
||||
cleanCommand = cleanCommand ?: branchConfig.cleanCommand,
|
||||
artifactDirs = artifactDirs ?: branchConfig.artifactDirs,
|
||||
stdoutLog = stdoutLog ?: branchConfig.stdoutLog,
|
||||
stderrLog = stderrLog ?: branchConfig.stderrLog,
|
||||
requirePullRequest = requirePullRequest ?: branchConfig.requirePullRequest,
|
||||
statusContext = statusContext ?: branchConfig.statusContext,
|
||||
docker =
|
||||
branchConfig.docker.copy(
|
||||
enabled = docker?.enabled ?: branchConfig.docker.enabled,
|
||||
image = docker?.image ?: branchConfig.docker.image,
|
||||
dockerfile = docker?.dockerfile ?: branchConfig.docker.dockerfile,
|
||||
context = docker?.context ?: branchConfig.docker.context,
|
||||
network = docker?.network ?: branchConfig.docker.network,
|
||||
env = docker?.env ?: branchConfig.docker.env,
|
||||
),
|
||||
)
|
||||
|
||||
companion object {
|
||||
/** Name of the implicit build that preserves the pre-ADR-0007 behavior: `onPush` over all branches. */
|
||||
const val DEFAULT = "default"
|
||||
|
||||
/** The result-pool name of [build] on [branch]: the plain branch for the default build. */
|
||||
fun poolName(
|
||||
branch: String,
|
||||
build: String,
|
||||
): String = if (build == DEFAULT) branch else "$branch@$build"
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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`.
|
||||
*
|
||||
* A definition with neither [onPush] nor [atTimes] never triggers automatically; that is
|
||||
* how `builds.default` is written when it is meant as a settings base only.
|
||||
*/
|
||||
data class TriggerConfig(
|
||||
/** Build every new commit of the selected branches. */
|
||||
val onPush: Boolean = false,
|
||||
/**
|
||||
@@ -28,7 +94,8 @@ data class BuildDefinition(
|
||||
val atTimes: List<String> = emptyList(),
|
||||
/**
|
||||
* Branch names or glob patterns (`*` matches any characters, also across `/`);
|
||||
* empty selects all origin branches.
|
||||
* empty selects all origin branches. A pattern prefixed with `!` excludes instead,
|
||||
* and an exclusion always wins — `["*", "!master"]` is every branch but master.
|
||||
*/
|
||||
val branches: List<String> = emptyList(),
|
||||
/**
|
||||
@@ -36,27 +103,15 @@ data class BuildDefinition(
|
||||
* empty applies no age filter. Combines with [branches] as an intersection.
|
||||
*/
|
||||
val activeWithin: String = "",
|
||||
/** Overrides the branch's build command; null inherits it. */
|
||||
val buildCommand: String? = null,
|
||||
/** Overrides the branch's clean command; null inherits it. */
|
||||
val cleanCommand: String? = null,
|
||||
/** Overrides the branch's artifact directories; null inherits them. */
|
||||
val artifactDirs: List<String>? = null,
|
||||
/** Overrides the branch's stdout log file name; null inherits it. */
|
||||
val stdoutLog: String? = null,
|
||||
/** Overrides the branch's stderr log file name; null inherits it. */
|
||||
val stderrLog: String? = null,
|
||||
/**
|
||||
* The watcher builds a selected branch only while its head commit matches a
|
||||
* pull-request head; null inherits. Pinned — a branch's own committed config can
|
||||
* never set it, or it would bypass its own gate.
|
||||
*/
|
||||
val requirePullRequest: Boolean? = null,
|
||||
/** Overrides of the branch's docker settings; null inherits them. */
|
||||
val docker: DockerOverrides? = null,
|
||||
) {
|
||||
/** True when [branch] matches the [branches] patterns (or none are configured). */
|
||||
fun selectsByName(branch: String): Boolean = branches.isEmpty() || branches.any { globToRegex(it).matches(branch) }
|
||||
/** True when [branch] matches the [branches] patterns (or none are configured) and none excludes it. */
|
||||
fun selectsByName(branch: String): Boolean {
|
||||
val (excluding, including) = branches.partition { it.startsWith(EXCLUDE_PREFIX) }
|
||||
if (excluding.any { globToRegex(it.removePrefix(EXCLUDE_PREFIX)).matches(branch) }) {
|
||||
return false
|
||||
}
|
||||
return including.isEmpty() || including.any { globToRegex(it).matches(branch) }
|
||||
}
|
||||
|
||||
/**
|
||||
* True when [branch] passes both selector parts; [headCommittedAt] is the branch
|
||||
@@ -80,35 +135,9 @@ data class BuildDefinition(
|
||||
|
||||
fun maxAge(): Duration = DurationParser.parse(activeWithin)
|
||||
|
||||
/** The branch settings with this build's overrides applied; unset values fall through. */
|
||||
fun applyTo(branchConfig: BranchConfig): BranchConfig =
|
||||
branchConfig.copy(
|
||||
buildCommand = buildCommand ?: branchConfig.buildCommand,
|
||||
cleanCommand = cleanCommand ?: branchConfig.cleanCommand,
|
||||
artifactDirs = artifactDirs ?: branchConfig.artifactDirs,
|
||||
stdoutLog = stdoutLog ?: branchConfig.stdoutLog,
|
||||
stderrLog = stderrLog ?: branchConfig.stderrLog,
|
||||
requirePullRequest = requirePullRequest ?: branchConfig.requirePullRequest,
|
||||
docker =
|
||||
branchConfig.docker.copy(
|
||||
enabled = docker?.enabled ?: branchConfig.docker.enabled,
|
||||
image = docker?.image ?: branchConfig.docker.image,
|
||||
dockerfile = docker?.dockerfile ?: branchConfig.docker.dockerfile,
|
||||
context = docker?.context ?: branchConfig.docker.context,
|
||||
network = docker?.network ?: branchConfig.docker.network,
|
||||
env = docker?.env ?: branchConfig.docker.env,
|
||||
),
|
||||
)
|
||||
|
||||
companion object {
|
||||
/** Name of the implicit build that preserves the pre-ADR-0007 behavior: `onPush` over all branches. */
|
||||
const val DEFAULT = "default"
|
||||
|
||||
/** The result-pool name of [build] on [branch]: the plain branch for the default build. */
|
||||
fun poolName(
|
||||
branch: String,
|
||||
build: String,
|
||||
): String = if (build == DEFAULT) branch else "$branch@$build"
|
||||
/** Marks a [branches] pattern as excluding. */
|
||||
const val EXCLUDE_PREFIX = "!"
|
||||
|
||||
private fun globToRegex(pattern: String): Regex =
|
||||
Regex(
|
||||
|
||||
@@ -79,6 +79,7 @@ class ConfigLoader(
|
||||
// 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 .gittally.yml of this branch", BRANCH_HINT)
|
||||
checkTriggerBlocks(branchLayer, "the committed .gittally.yml of this branch", BRANCH_HINT)
|
||||
return toConfig(deepMerge(loadRaw(workingDir), stripPinned(branchLayer)))
|
||||
}
|
||||
|
||||
@@ -200,8 +201,34 @@ class ConfigLoader(
|
||||
}
|
||||
|
||||
private fun isTriggered(definition: Any?): Boolean {
|
||||
val entry = definition as? Map<*, *> ?: return false
|
||||
return entry["onPush"] == true || (entry["atTimes"] as? List<*>)?.isNotEmpty() == true
|
||||
val trigger = (definition as? Map<*, *>)?.get("trigger") as? Map<*, *> ?: return false
|
||||
return trigger["onPush"] == true || (trigger["atTimes"] as? List<*>)?.isNotEmpty() == true
|
||||
}
|
||||
|
||||
/**
|
||||
* Refuses a definition that still writes its trigger and selector keys flat instead of
|
||||
* inside `trigger`. Silently ignoring them would leave a build with no trigger at all —
|
||||
* a branch that stops building without saying so, which is worse than not starting.
|
||||
* Scoped like [checkVersion]: per file, so the message names the one to fix.
|
||||
*/
|
||||
private fun checkTriggerBlocks(
|
||||
raw: Map<String, Any?>,
|
||||
source: String,
|
||||
hint: String,
|
||||
) {
|
||||
val builds = raw["builds"] as? Map<*, *> ?: return
|
||||
val offenders =
|
||||
builds.entries.mapNotNull { (name, value) ->
|
||||
val flat = (value as? Map<*, *>)?.keys?.filter { it in FLAT_TRIGGER_KEYS } ?: return@mapNotNull null
|
||||
flat.takeIf { it.isNotEmpty() }?.let { "builds.$name: ${it.joinToString(", ")}" }
|
||||
}
|
||||
if (offenders.isEmpty()) {
|
||||
return
|
||||
}
|
||||
throw ConfigFormatException(
|
||||
"$source declares a build trigger outside its trigger block (${offenders.joinToString("; ")}). " +
|
||||
"Move these keys into a `trigger:` block inside the definition. $hint",
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -213,7 +240,7 @@ class ConfigLoader(
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
private fun mergeBuildDefaults(raw: Map<String, Any?>): Map<String, Any?> {
|
||||
val builds = raw["builds"] as? Map<String, Any?> ?: return raw
|
||||
val base = (builds[BuildDefinition.DEFAULT] as? Map<String, Any?>)?.minus(SELECTOR_KEYS) ?: return raw
|
||||
val base = (builds[BuildDefinition.DEFAULT] as? Map<String, Any?>)?.minus(TRIGGER_KEYS) ?: return raw
|
||||
if (base.isEmpty()) {
|
||||
return raw
|
||||
}
|
||||
@@ -245,6 +272,8 @@ class ConfigLoader(
|
||||
// per file, so the message names the file to fix — the merged map has no provenance
|
||||
checkVersion(project, ".gittally.yml", ROLLBACK_HINT)
|
||||
checkVersion(repoInstall, ".git/gittally/.gittally.yml", ROLLBACK_HINT)
|
||||
checkTriggerBlocks(project, ".gittally.yml", ROLLBACK_HINT)
|
||||
checkTriggerBlocks(repoInstall, ".git/gittally/.gittally.yml", ROLLBACK_HINT)
|
||||
return deepMerge(project, repoInstall)
|
||||
}
|
||||
|
||||
@@ -348,15 +377,25 @@ class ConfigLoader(
|
||||
/**
|
||||
* Settings keys a branch must never override, in a build definition as well as in
|
||||
* a legacy branch entry: the trust gate that decides whether the watcher builds
|
||||
* this branch at all.
|
||||
* this branch at all, and the Gitea check this build reports as — a branch that
|
||||
* could choose its own context could take over the check a branch protection
|
||||
* rule depends on.
|
||||
*/
|
||||
private val PINNED_SETTING_KEYS = setOf("requirePullRequest")
|
||||
private val PINNED_SETTING_KEYS = setOf("requirePullRequest", "statusContext")
|
||||
|
||||
/** `docker` keys a branch must never override: the sandbox policy. */
|
||||
private val PINNED_DOCKER_KEYS = setOf("enabled", "network")
|
||||
|
||||
/** Keys of a build definition that say *when* it runs; never inherited from `builds.default`. */
|
||||
private val SELECTOR_KEYS = setOf("onPush", "atTimes", "branches", "activeWithin")
|
||||
/**
|
||||
* The one key of a build definition that says *when* and *for which branches* it
|
||||
* runs; never inherited from `builds.default`. A single key on purpose: a selector
|
||||
* added inside it is non-inheritable by construction, where a list of key names
|
||||
* would have to be remembered.
|
||||
*/
|
||||
private val TRIGGER_KEYS = setOf("trigger")
|
||||
|
||||
/** 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 const val LEGACY_BRANCHES_WARNING = "legacy-branches-ignored"
|
||||
|
||||
|
||||
@@ -44,10 +44,23 @@ sealed interface VersionVerdict {
|
||||
}
|
||||
|
||||
/** A configuration file this GitTally must not read; carries the file's name in its message. */
|
||||
class ConfigVersionException(
|
||||
open class ConfigException(
|
||||
message: String,
|
||||
) : RuntimeException(message)
|
||||
|
||||
/** The file declares a GitTally that cannot read it, see [ConfigVersions]. */
|
||||
class ConfigVersionException(
|
||||
message: String,
|
||||
) : ConfigException(message)
|
||||
|
||||
/**
|
||||
* The file is written in a shape this GitTally no longer reads. Refusing it is the point:
|
||||
* a key that moved and is silently ignored means a build that quietly stops happening.
|
||||
*/
|
||||
class ConfigFormatException(
|
||||
message: String,
|
||||
) : ConfigException(message)
|
||||
|
||||
object ConfigVersions {
|
||||
/**
|
||||
* The version in which the configuration format last changed incompatibly — a file
|
||||
|
||||
@@ -22,7 +22,7 @@ data class GitTallyConfig(
|
||||
) {
|
||||
/** The configured [buildDefinitions] plus the implicit `default` build unless overridden. */
|
||||
fun effectiveBuildDefinitions(): Map<String, BuildDefinition> =
|
||||
mapOf(BuildDefinition.DEFAULT to BuildDefinition(onPush = true)) + buildDefinitions
|
||||
mapOf(BuildDefinition.DEFAULT to BuildDefinition(trigger = TriggerConfig(onPush = true))) + buildDefinitions
|
||||
|
||||
/**
|
||||
* The settings one build of [build] on [branch] runs with: the branch entry (falling
|
||||
@@ -153,6 +153,8 @@ data class BranchConfig(
|
||||
* head (`refs/pull/<n>/head` on origin); manual `build` commands are not affected.
|
||||
*/
|
||||
val requirePullRequest: Boolean = false,
|
||||
/** Gitea commit status context of this build; empty uses `gitea.statusContext`. */
|
||||
val statusContext: String = "",
|
||||
val autoBuild: AutoBuildConfig = AutoBuildConfig(),
|
||||
val docker: DockerConfig = DockerConfig(),
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user