# 0056. `TalosCluster.spec.gitops`: a repository connection, strict mode, and the UI/CLI opening PRs

Status: Superseded by ADR 0101 · Date: 2026-10-26 · Design section: GitOps and Argo CD
Decided: `TalosCluster.spec.gitops` carries `mode` (`relaxed` default, `strict`) and an optional `repository` connection (host, https API URL, owner/repo, https clone URL, base branch, a `tokenSecretRef` read in the UI backend's own namespace, `machinesPath` required for strict); `Server.requireUntracked` becomes `planWrite`, which sends a CR Argo CD tracks to a pull request when a connection exists and refuses otherwise, and sends a strict approval to a pull request that adds the `TalosMachine` file; changing `mode` rewrites nothing; the response carries `pull_request_url` and the audit action gets a `-pr` suffix; amended 2026-10-26: the PR branch is `ichor/<verb>-<name>` (since 2026-10-27 `ichor/<verb>-<namespace>-<name>`) and a request whose head branch already has an open PR returns that PR instead of opening another, and deleting a tracked `TalosConfigPatch` becomes a PR that removes its document (or its file)

## Context

ADR 0051 made every UI/CLI write refuse a tracked CR because no repository connection could be expressed.
`pkg/prmode` (ADR 0037, 0038) could already clone, edit and open a PR, and `Decide` already named
`ModeOpenPR`, but nothing fed it a connection, and nothing handled strict mode (architecture.md: "approving a
node *is* a PR that adds the `TalosMachine`").

## Decisions

**1. The shape.** `spec.gitops.mode` is `relaxed` (default) or `strict`. `spec.gitops.repository` is the
connection: `host` (`github|gitlab|gitea`), `apiURL`, `owner`, `repo`, `cloneURL`, `baseBranch` (default
`main`), `tokenSecretRef` (a `SecretKeySelector`) and `machinesPath`. CEL and patterns reject what can be
rejected at admission: both URLs must be `https://` and carry no `@` (so no credentials in a CR that lives in
Git, and the token is never sent in clear text); `owner`/`repo`/`machinesPath` are patterns; `machinesPath` may
not contain `.` or `..` segments; `strict` needs `repository` with `machinesPath`. The Go side re-checks the
new-file path anyway (`prmode.SubmitPR` refuses an absolute path, `..`, `.git/` and an existing file).

The token is a reference, never a value. The Secret is read in the UI backend's own namespace
(`Server.ControllerNamespace`), never `cluster.Namespace` (ADR 0011), through the namespaced
`ichor-ui-secrets-role` that already exists for the secrets bundle; an empty namespace is refused rather than
read as "all". No RBAC was added for the token. `get` on `applications.argoproj.io` was added (cluster scoped,
like ADR 0051's rules, because Argo CD's namespace is configurable): PR mode reads the one Application named by
the CR's own tracking marker (`gitopstracking.ArgoApplicationOf`), never lists.

**2. Changing `mode`.** It is a mutable field and changing it rewrites nothing. `relaxed` -> `strict`: the
`TalosMachine`s already written directly stay in the cluster and are not in Git; nothing prunes or reverts them.
From then on a new approval is a PR. `ichor gitops export` is how existing ones reach Git. `strict` -> `relaxed`:
a PR already open stays open; new approvals write directly. Mode is read on each call, so there is no state to
migrate. The design says nothing more, and a controller that "fixes" the difference would break "the controller
never mutates `spec`".

**3. Strict approval.** `ApproveMachine` runs every check it always ran (already approved, weak identity,
role), then, instead of `Update`, calls `SubmitPR` with a new `prmode.NewFile`: the approved `TalosMachine`
(`apiVersion`, `kind`, `name`, `namespace`, `spec`; no status, no server metadata) at
`<machinesPath>/<name>.yaml`. `Locate` has nothing to find for a CR that is not in Git, so `SubmitPRRequest` got
a `NewFile` branch that skips `Locate`/`Patch` and refuses an existing file. The pending `TalosMachine` in the
cluster is not written: it stays pending until the PR merges and Argo CD applies the file over it. A
`TalosMachine` Argo CD already tracks takes the edit-in-place path of decision 4 in either mode. Rejecting an
untracked pending machine is not an "add": a rejection records a discovery decision and is written directly, in
both modes. Strict applies to approval only.

**4. Relaxed, tracked CR.** `planWrite` runs `IsTracked`, then `Decide(by, repository != nil)`:
`ModeDirectWrite` writes as before; `ModeOpenPR` finds the Application, requires `ClassifySource` to return
`SourceFile`, requires its `repoURL` to be the connection's `cloneURL` (a different repository would be pushed
with the wrong token), uses the Application's `targetRevision` as the base branch (`HEAD`/empty falls back to
`baseBranch`), and opens the PR; `ModeReadOnly` refuses with ADR 0051's message. Edits: approve/reject set
`spec.approvalState`, `approvalDecision`, and `role`/`hostname`/`nodeLabels` when set (`prmode.Patch` now adds a
missing leaf key; a missing parent is still an error); `UpdatePatch` replaces the file's `spec` node whole,
matching its full-replace semantics, merged into the existing node (`FieldUpdate.Merge`) so comments and formatting on unchanged fields are kept; a comment on a removed key or item goes with it. `DeletePatch` of a tracked
patch is a PR too (decision 6). Flux stays read-only (ADR 0037). A cluster that cannot be read (not found) is "relaxed, no
repository" so an untracked write is never newly blocked; any other read error refuses.

The PR description carries the requester and an impact section: for a patch, its layer/target and
`fieldmeta.Assess`'s reboot / install-time-only / unsupported (the `PreviewImpact` rules); for a machine, the
affected node and what approving starts. The response carries `pull_request_url`
(`ApproveMachineResponse`, `RejectMachineResponse`, `UpdatePatchResponse`); `machine`/`patch` in it are the
unchanged stored object. The audit record's action becomes `approve-pr`, `reject-pr` or `update-pr`. The CLI
already prints the whole response, so it shows the URL; the UI shows a notice with a link (http(s) only) and no
screen follows the PR.

**5. Requester.** `AuditRecord.actor` is still the fixed `ichor-ui` / `ichor-cli` string (OIDC is phase 9). The
PR says "Requested by: ichor-ui". When OIDC lands the same value will be the user, with no change here.

**6. One open PR per change, and deleting by PR (amendment).** The first version named branches
`ichor/<verb>-<name>-<unix time>`, so every click on a still-pending machine opened another PR that conflicted
with the first on merge.

- *Identity.* The branch is now `ichor/<verb>-<name>` (`approve`, `reject`, `update`, `delete`). Before cloning,
  `prmode.SubmitPR` asks the host for an open PR with that head and the same base (`FindOpenPullRequest`); if
  there is one it returns its URL and pushes nothing. The UI needs no change for this: the response already
  carries `pull_request_url`, so the existing notice shows the URL of the PR that is already open. No screen
  lists open PRs.
- *A second request with different content* (approving again with another role) gets the open PR back; it is
  not amended or force-pushed under a reviewer. To change it, close that PR (or edit its branch) and ask again.
  The PR body keeps the request it was opened with.
- *A lookup that fails is an error*, never "none", since "none" would open a duplicate. Its error text is
  scrubbed of the token like any other.
- *A branch left behind* (the earlier PR was closed or merged and the branch kept) has no open PR, so the push
  that follows replaces it with `--force`. The `ichor/` branches are Ichor's own; a person's work does not
  belong on one.
- *Host calls* (verified against the docs 2026-10-26). GitHub: `GET /repos/{owner}/{repo}/pulls?state=open&head={owner}:{branch}&base={base}`
  (docs.github.com/en/rest/pulls/pulls). GitLab: `GET /api/v4/projects/:id/merge_requests?state=opened&source_branch=&target_branch=`
  (docs.gitlab.com/api/merge_requests). Gitea/Forgejo: the list endpoint has no head filter, so `GET
  /api/v1/repos/{owner}/{repo}/pulls/{base}/{head}` ("get a pull request by base and head", 404 when none;
  docs.gitea.com/api/1.22). Anything not `open` counts as none. Same `net/http` shape as the create calls.
- *Deleting.* `SubmitPRRequest.Delete` locates the file as an edit does and `prmode.RemoveDocument` drops the
  one matching document with its `---`; every other byte, comments and spacing included, stays as it was. When
  nothing but comments is left, the file is removed (`git rm`). `DeletePatch` returns `pull_request_url`
  (`DeletePatchResponse` field 2, audit action `delete-pr`) and writes nothing to the cluster. Whether Argo CD
  prunes the live object after the merge is the Application's setting. The UI's delete dialog shows the notice
  in place of navigating away.

## Token handling

The token goes to git only through `GIT_CONFIG_*` `http.extraHeader` in the environment (`prmode.gitAuthEnv`) and
to the host as a request header. Errors from `SubmitPR` are scrubbed of the token and its base64 Basic form
before they are returned; the Secret read errors name the Secret, never its data. Tests drive a host that echoes
the `Authorization` header on failure, a failing clone, and the success path, and look for the token in the
error, the PR body, the response and the pushed file.

## Consequences

- Until a strict-approval PR merges the machine stays pending and the UI still offers approval; a click now
  returns the open PR's URL (decision 6). Nothing follows a PR to merge, and the UI does not mark a machine
  as having one open.
- Amended 2026-10-27: the branch is `ichor/<verb>-<namespace>-<name>` (`prBranchName`), so same-named objects in
  different namespaces no longer share one; a name with characters outside `[a-z0-9-]` or over 100 bytes gets an
  8-hex hash of the full name appended, which keeps it a valid, bounded ref. PRs opened under the old name are not found.
- `machinesPath` is one directory for every cluster's machines in that connection; a layout that needs more is
  later work.
- The new-file manifest carries no labels or annotations from the live `TalosMachine`.
- A merged strict PR relies on Argo CD applying over the existing pending `TalosMachine`; if the Application
  prunes or uses `Replace`, that is the Application's setting, not Ichor's.

(Amended 2026-10-26, ADR 0065: a request may carry `Override{reason}` to write a tracked or strict-mode CR directly and still open this PR, if Kubernetes RBAC grants the `override` verb.)