Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
63a58d6
test(git): reproduce a new write branch created on a stale parent
sunib Oct 1, 2026
6ac4ba3
fix(git): check the parent before creating a write branch, and create…
sunib Oct 1, 2026
b0fc1d1
feat(api): add GitTarget spec.parentBranch, the branch a new write br…
sunib Oct 1, 2026
55e3eb5
fix(git): rebuild retained writes on a parent change, and scope the p…
sunib Oct 1, 2026
b948227
feat(api): make spec.parentBranch immutable, and report status.remote…
sunib Oct 1, 2026
bfbb7d9
feat(api): report whether the parent was found, missing or unborn
sunib Oct 1, 2026
dcbc8b8
fix(git): keep a parent change pending until the rebuild succeeds
sunib Oct 1, 2026
015536b
docs: record where the parent-branch work goes next, and the open #40…
sunib Oct 1, 2026
73a7e61
docs: keep the parent-branch pages to what is true, and set the merge…
sunib Oct 1, 2026
0474b34
fix(commit): never fail a CommitRequest the worker still holds, and w…
sunib Oct 1, 2026
a7eccb2
fix(commit): withdraw a CommitRequest from the worker that holds it, …
sunib Oct 1, 2026
3fb6666
fix(git): scope base trust to the parent generation and admit a push …
sunib Oct 1, 2026
9da39b2
fix(git): never orphan a branch in a repository that is not empty
sunib Oct 1, 2026
c44732a
fix(git): make recovery from a missing parent an obligation the worke…
sunib Oct 1, 2026
54d5e68
fix(git): follow a switched default branch, and replay onto a write b…
sunib Oct 1, 2026
8dd4d06
docs: fold the hardening pass into the parent-branch contract, and pi…
sunib Oct 1, 2026
93120ca
docs: describe the safety window as a withdrawal request, and fix the…
sunib Oct 1, 2026
504ab7c
fix(git): keep a recovery obligation for work dropped in a second out…
sunib Oct 1, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .coverage-baseline
Original file line number Diff line number Diff line change
@@ -1 +1 @@
79.0
79.4
62 changes: 62 additions & 0 deletions api/v1alpha3/gittarget_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.gitProviderRef) || self.gitProviderRef == oldSelf.gitProviderRef",message="spec.gitProviderRef is immutable; delete and recreate the GitTarget to change its destination"
// +kubebuilder:validation:XValidation:rule="self.branch == oldSelf.branch",message="spec.branch is immutable; delete and recreate the GitTarget to change its destination"
// +kubebuilder:validation:XValidation:rule="self.path == oldSelf.path",message="spec.path is immutable; delete and recreate the GitTarget to change its destination"
// +kubebuilder:validation:XValidation:rule="has(self.parentBranch) == has(oldSelf.parentBranch) && (!has(self.parentBranch) || self.parentBranch == oldSelf.parentBranch)",message="spec.parentBranch is immutable; delete and recreate the GitTarget to change it"
//
// spec.clusterProviderRef names the SOURCE cluster a GitTarget mirrors FROM (see its field doc). It
// is immutable — a folder's source cluster is part of what the folder means, like
Expand All @@ -49,6 +50,25 @@ type GitTargetSpec struct {
// +kubebuilder:validation:MinLength=1
Branch string `json:"branch"`

// Why "parent" and not base/source/revision: docs/definitions.md reserves those words, and the
// field is flat because it is one fact. It is immutable like branch and path: it decides what
// history a new write branch gets, and a parent changed under a worker holding writes is the
// one case that needed its own replay. Delete and recreate is cheap, because the destination is
// unchanged. It can never hold a commit or tag: a pinned starting point would be a sibling field.

// ParentBranch is the branch the write branch (spec.branch) is created from, and the branch the
// target compares against while the write branch does not exist on the remote. Omitted: the
// branch the remote's HEAD points at (its default branch), or a branch with no history when
// the repository is empty. When set, it must exist on the remote for the write branch to be
// created; otherwise the GitTarget reports ParentBranchNotFound and writes nothing. Once the
// write branch exists, it is used as it is and the parent is not followed. It is only read, so
// it does not need to be in the GitProvider's allowedBranches. All GitTargets on one GitProvider
// and branch must name the same parent branch (omitted counts as its own value).
// Immutable, including adding or removing it: delete and recreate the GitTarget to change it.
// +optional
// +kubebuilder:validation:MinLength=1
ParentBranch string `json:"parentBranch,omitempty"`

// Path within the repository to write resources to, relative to the repository
// root. Required and must be non-empty — there is no default, so a GitTarget can
// never silently write to the repository root. To deliberately target the
Expand Down Expand Up @@ -457,6 +477,48 @@ type GitTargetRemoteStatus struct {
// +optional
// +kubebuilder:validation:Enum=Push;Fetch
VerifiedBy string `json:"verifiedBy,omitempty"`

// Parent is what the write branch would be created from, observed with it: present only
// while the remote does not carry the write branch. Once the branch exists its parent is not
// followed, so this is omitted.
// +optional
Parent *GitTargetParentStatus `json:"parent,omitempty"`
}

// GitTargetParentState is what a write branch that does not exist yet would be created from.
// +kubebuilder:validation:Enum=Found;Missing;Unborn
type GitTargetParentState string

const (
// GitTargetParentFound means the parent branch exists; the first commit builds on its tip.
GitTargetParentFound GitTargetParentState = "Found"
// GitTargetParentMissing means the parent is not on the remote either, so the write branch
// cannot be created; the GitTarget reports ParentBranchNotFound. The parent is spec.parentBranch,
// or, when that is omitted, the default branch of a repository that is not empty but whose HEAD
// names no branch it carries.
GitTargetParentMissing GitTargetParentState = "Missing"
// GitTargetParentUnborn means the repository is empty, with no refs at all (tags included), and
// no parent is configured, so the first commit starts the write branch with no history.
GitTargetParentUnborn GitTargetParentState = "Unborn"
)

// GitTargetParentStatus is the parent branch of a write branch that does not exist yet.
type GitTargetParentStatus struct {
// State is what the write branch would be created from: Found (the parent's tip), Missing
// (the parent is not on the remote; nothing is written) or Unborn (an empty repository, with no
// refs at all, and no parent configured; the first commit has no history).
// +required
State GitTargetParentState `json:"state"`

// Branch is the parent branch: spec.parentBranch, or the branch the remote's HEAD names when
// that is omitted. Empty when State is Unborn, and when the remote names no default branch.
// +optional
Branch string `json:"branch,omitempty"`

// Commit is the commit hash the parent branch is at on the remote: the commit the write
// branch's first commit would build on. Set only when State is Found.
// +optional
Commit string `json:"commit,omitempty"`
}

// Two rules for anything added here. A field earns its place only if a reader cannot get it from
Expand Down
20 changes: 20 additions & 0 deletions api/v1alpha3/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 46 additions & 0 deletions config/crd/bases/configbutler.ai_gittargets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,19 @@ spec:
- Ignore
- PushEmptyCommit
type: string
parentBranch:
description: |-
ParentBranch is the branch the write branch (spec.branch) is created from, and the branch the
target compares against while the write branch does not exist on the remote. Omitted: the
branch the remote's HEAD points at (its default branch), or a branch with no history when
the repository is empty. When set, it must exist on the remote for the write branch to be
created; otherwise the GitTarget reports ParentBranchNotFound and writes nothing. Once the
write branch exists, it is used as it is and the parent is not followed. It is only read, so
it does not need to be in the GitProvider's allowedBranches. All GitTargets on one GitProvider
and branch must name the same parent branch (omitted counts as its own value).
Immutable, including adding or removing it: delete and recreate the GitTarget to change it.
minLength: 1
type: string
path:
description: |-
Path within the repository to write resources to, relative to the repository
Expand Down Expand Up @@ -521,6 +534,10 @@ spec:
- message: spec.path is immutable; delete and recreate the GitTarget to
change its destination
rule: self.path == oldSelf.path
- message: spec.parentBranch is immutable; delete and recreate the GitTarget
to change it
rule: has(self.parentBranch) == has(oldSelf.parentBranch) && (!has(self.parentBranch)
|| self.parentBranch == oldSelf.parentBranch)
- message: spec.clusterProviderRef is immutable; delete and recreate the
GitTarget to change the cluster it mirrors
rule: self.clusterProviderRef == oldSelf.clusterProviderRef
Expand Down Expand Up @@ -682,6 +699,35 @@ spec:
an old value there means the layout has not changed rather than that nothing has looked.
format: date-time
type: string
parent:
description: |-
Parent is what the write branch would be created from, observed with it: present only
while the remote does not carry the write branch. Once the branch exists its parent is not
followed, so this is omitted.
properties:
branch:
description: |-
Branch is the parent branch: spec.parentBranch, or the branch the remote's HEAD names when
that is omitted. Empty when State is Unborn, and when the remote names no default branch.
type: string
commit:
description: |-
Commit is the commit hash the parent branch is at on the remote: the commit the write
branch's first commit would build on. Set only when State is Found.
type: string
state:
description: |-
State is what the write branch would be created from: Found (the parent's tip), Missing
(the parent is not on the remote; nothing is written) or Unborn (an empty repository, with no
refs at all, and no parent configured; the first commit has no history).
enum:
- Found
- Missing
- Unborn
type: string
required:
- state
type: object
verifiedBy:
description: |-
VerifiedBy is what proved it: `Push` means the server accepted a ref update of ours, so
Expand Down
8 changes: 6 additions & 2 deletions docs/INDEX.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,14 @@ decided), **design, decided** (decision made, not built), **partly built**, **bu
**deferred** (parked, kept as a decision record). The label is the first thing in the page,
so you never have to read a proposal to find out it already shipped.

### Open — 23 pages
### Open (27 pages)

| Doc | Open question |
|---|---|
| [`gittarget-parent-branch.md`](design/gittarget-parent-branch.md) | **built in #407, not merged yet.** `spec.parentBranch`: a target stays on standby until a write needs a branch, then creates its write branch from the parent's tip as checked at publication. The contract as built, the defect it started from, prior art and naming. Open: parent per `GitProvider.status.branches` entry. |
| [`gittarget-parent-hardening.md`](design/gittarget-parent-hardening.md) | **done in #407.** Parent changes on a live worker, an empty repository gaining a ref, an unresolved default branch, recovery that cannot strand work, and the refused upload, without adding a pull to the fast path; plus the two recovery fixes from the review at `93120caf`. Records the merge boundary and the next separate PRs. |
| [`gittarget-parent-empty-repository.md`](design/gittarget-parent-empty-repository.md) | **deferred.** An empty repository's default branch: the server reports it over protocol v2 but go-git discards it (upstream fix first); then a separate policy decision on which branch the first commit may create. Assuming `main` was considered and rejected. |
| [`gittarget-parent-observation.md`](design/gittarget-parent-observation.md) | **deferred.** Keep the parent visible after the write branch exists: availability first, ancestry only after measuring how often a shallow checkout can answer it. Informational; authorizes no branch mutation. |
| [`gittarget-red-status-plan.md`](design/gittarget-red-status-plan.md) | **design, partly built.** The smaller alternative to Git write preflight: make a `GitTarget` turn red and explain why no commit was made, without moving Git checks into the admission webhook and without adding a second state source. Keeps the existing kstatus trio and domain conditions, ranks the most likely operator-facing failures, and has now shipped the first status-transition slice: refusal messages republish when details change, scoped refusals cannot be cleared by unrelated scope success, `BlockMessage()` carries stable issue kind/path/actor detail, and the unsupported-folder e2e covers Git-side break and recovery. |
| [`open-asks-priority.md`](design/open-asks-priority.md) | **the work queue.** Merges three overlapping backlogs — the downstream consumer asks, the API-surface block left unbuilt by the status and configuration-model review, and the config-surface proposal (B1–B6) — into one ordered queue under four stated tests, and says where we deliberately do **not** do what was asked. The standing caveat narrowed once the layout model reversed: a Tier 2 entry belongs to postponed [#294](https://github.com/ConfigButler/gitops-reverser/issues/294) only if it breaks a `GitTarget` field, and everything else is independently schedulable. Makes one design call against what was asked: **delete Option C sibling inference** rather than ship an off-switch for it, because it let a human's edit to the repository change operator behavior with nothing in status recording the move. That deletion has shipped, and "what the deletion taught" records what building it found. **F9 is Tier 1**: the only item whose answer is unknown rather than whose work is unscheduled, and it gates planning the enum work |
| [`proving-the-metric-surface.md`](design/proving-the-metric-surface.md) | **design, partly built.** What identity an operator needs to locate a failure, and what an e2e check has to do to be worth trusting. Written after a metric value that was declared, switched on, surfaced as a condition reason and counted turned out to have no producer at all, and the check added to catch that class then skipped whenever the counter read zero — so deleting every recording call left the suite green. Two rules follow: an assertion must not take its trigger from the metric under test, and no enum value may be reachable only through an injected fake. Carries a cardinality policy that separates bounded configuration identity from per-request identity, a table of which instrument families should own `source_cluster` directly rather than by join, and the PromQL reasoning for `increase()` versus `max_over_time` (neither applies mechanically: one cannot see an event before the first sample, the other cannot reconstruct totals across a reset). Steps 1–4 are built; step 5, diagnostics export, the `e2e_run` scrape label and a pinned-seed regression are not |
Expand All @@ -152,7 +156,7 @@ so you never have to read a proposal to find out it already shipped.
| [`sensitive-resource-diagnostics-follow-up.md`](design/sensitive-resource-diagnostics-follow-up.md) | deferred diagnostics |
| [`e2e-git-server-choice.md`](design/e2e-git-server-choice.md) | stay on Gitea or move to Forgejo — the `_csrf` pin is fixable in place on both, so the migration is now a preference call, not a fix; also why we adopt no SDK either way |

### Built, and kept here anyway — 6 pages
### Built, and kept here anyway (6 pages)

These have shipped. They stay in `design/` under the exception above, because Go source
cites them by path as the rationale for what the code does, and `finished/` declares
Expand Down
50 changes: 50 additions & 0 deletions docs/TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,58 @@

This file is meant to track the smaller current backlog, not historical notes.

## Follow-ups from #404 (commit window surface)

PR #404 is merged; these need fix PRs against main, in this order. (Its CommitRequest timeout
finding is fixed in #407: the controller no longer fails a request the worker holds.)

**Next, in one fix PR:**

- [ ] **A suspended target makes a `CommitEmpty` save report Ready=True with no commit.**
`recordCommitRequest` returns `false, nil` on `errTargetSuspended`; the design table
(`docs/design/commit-timing-surface.md`) says a suspended target is a failure. An attached window
that changed nothing resolves `AlreadyPresent` the same way. Decide: fail the save, or change the
table.
- [ ] **The worker records a `CommitEmpty` save while the render-fidelity gate is closed.** Found by
reading; reproduce first. `recordCommitRequest` never calls `normalWritesAllowed`, so a save whose events the
gate dropped records "no writes were seen". The attached-window path fails the request instead.

**Before the next release** (docs only):

- [ ] `docs/UPGRADING.md`, the v1alpha3 window entry:
- step 1: tell readers to suspend the Flux or Argo source of a GitOps-managed GitTarget (or remove
the field there) until step 4; otherwise that tool re-applies the string `commit.window` and
breaks LIST;
- the Was/Is table needs a row for the chart value `quickstart.gitTarget.commit.window`, now an
object;
- "a removed `closeDelay` is not a hazard" is too strong: the API server prunes it silently for
programmatic clients (see `duration_fields_admission_test.go`).
- [ ] Descriptions that still say a save "closes the window now": the CommitRequest godoc and CRD
description, `docs/configuration.md` (lines 14, 32, 123), and the GitTarget `Commit` godoc, which
also omits the 1m `maxDuration` default. `docs/spec/commitrequest-design.md:126` still says
`NoOpenWindow` and "close deadline".

**Later, non-blocking:**

- [ ] `expireWaitingCommitRequests` ranges over a map, so empty commits for saves that time out in
the same pass land in random order; sort by `seq`.
- [ ] `buildRequestRecordWrite` keeps only the username; carry the full `UserInfo` so the record has
the same author identity as a window commit.
- [ ] A record commit skips `requestTemplate`; fix the code or the doc comment that says it is phrased
like every other commit.
- [ ] Nits: a stale doc comment stacked on `commitWindowFor`; kstatus test names that still say
"close-delay".

## Current backlog

- [ ] After #407 and the #404 fixes above, take the first
[parent-observation step](design/gittarget-parent-observation.md#implementation-and-validation):
retain parent availability and its timestamp after the write branch exists. Use existing
advertisements, with no extra connections or changes to readiness or publication. Keep ancestry
classification and empty-repository bootstrap policy in separate changes. The
[merge review](design/gittarget-parent-hardening.md#merge-review-at-93120caf) records the two
recovery fixes made before #407 merged.

- [ ] Finish making `GitTarget` turn red quickly and explain why no commit was made.
The plan is in [gittarget-red-status-plan.md](design/gittarget-red-status-plan.md). This is the
replacement direction for Git write preflight: no admission webhook Git work, no extra state
Expand Down
Loading
Loading