feat(api): expose deletionPolicy so CRs are safe to rename #109

Merged
sean merged 1 commit from feat/deletion-policy into trunk 2026-08-07 08:47:29 +00:00
Owner

Renaming a ForgejoRepository is a delete plus a create, and the delete's finalizer called
DeleteRepo() — destroying all code, refs and history. That single fact is why 56 gitmirror CR
names are frozen with a stale -alice-<5char> stem and why the fleet Kustomization runs
prune: false forever. A name should not be un-editable because the operator has no way to say
"release this CR, keep the thing".

Almost none of this is new. libseanfarm-operator's reconcile harness already implements
DeletionPolicy (Retain|Delete) and already skips the remote delete for Retain, via the
optional DeletionPolicyProvider interface on the object. openbao and minio already
implement it. The Forgejo types simply never exposed the field — and objects that don't
implement the interface are treated as always-Delete.

Defaults follow what the remote state is, not the kind's name. Retain for the
irreplaceable: repository (code + history) and team (membership and permissions); also
user and organization, whose Delete() already returned nil, so the field just makes
existing behaviour legible. Delete for grants, credentials and config — a collaborator grant
or access token that outlives its CR is a security residue, so uniform Retain would be the
less safe choice.

Behaviour change: deleting a ForgejoRepository CR no longer deletes the repository
unless deletionPolicy: Delete is set. The gitmirror decommission runbook needs updating.

Each accessor re-encodes its default for the empty string as well as via
+kubebuilder:default. Both are required: kubebuilder defaults apply at write time, so the
56 CRs already stored read back empty and would otherwise fall through to the harness default of
Delete — destroying precisely the state this field exists to protect.

The coverage test derives the kind set from the scheme rather than a hand-written list, so a
CRD added later cannot inherit always-Delete silently; the per-kind defaults stay a
hand-written table because that table is the spec. Mutation-tested both ways — flipping the
repository default to Delete, and removing an accessor entirely — each is caught.

make test green (incl. envtest); 14/14 CRDs carry the field with the library's enum.

Renaming a `ForgejoRepository` is a delete plus a create, and the delete's finalizer called `DeleteRepo()` — destroying all code, refs and history. That single fact is why 56 gitmirror CR names are frozen with a stale `-alice-<5char>` stem and why the fleet Kustomization runs `prune: false` forever. A name should not be un-editable because the operator has no way to say *"release this CR, keep the thing"*. **Almost none of this is new.** `libseanfarm-operator`'s reconcile harness already implements `DeletionPolicy` (`Retain`|`Delete`) and already skips the remote delete for `Retain`, via the optional `DeletionPolicyProvider` interface on the **object**. openbao and minio already implement it. The Forgejo types simply never exposed the field — and objects that don't implement the interface are treated as always-`Delete`. **Defaults follow what the remote state *is*, not the kind's name.** `Retain` for the irreplaceable: `repository` (code + history) and `team` (membership and permissions); also `user` and `organization`, whose `Delete()` already returned `nil`, so the field just makes existing behaviour legible. `Delete` for grants, credentials and config — a collaborator grant or access token that outlives its CR is a **security residue**, so uniform `Retain` would be the *less* safe choice. ⚠ **Behaviour change:** deleting a `ForgejoRepository` CR no longer deletes the repository unless `deletionPolicy: Delete` is set. The gitmirror decommission runbook needs updating. Each accessor re-encodes its default for the empty string *as well as* via `+kubebuilder:default`. Both are required: kubebuilder defaults apply at **write** time, so the 56 CRs already stored read back empty and would otherwise fall through to the harness default of `Delete` — destroying precisely the state this field exists to protect. The coverage test derives the kind **set** from the scheme rather than a hand-written list, so a CRD added later cannot inherit always-`Delete` silently; the per-kind defaults stay a hand-written table because that table is the spec. Mutation-tested both ways — flipping the repository default to `Delete`, and removing an accessor entirely — each is caught. `make test` green (incl. envtest); 14/14 CRDs carry the field with the library's enum.
feat(api): expose deletionPolicy so CRs are safe to rename
All checks were successful
pipeline/ci CI green @ bd871feafa38
bd871feafa
Renaming a ForgejoRepository is a delete plus a create, and the delete's
finalizer called DeleteRepo() -- destroying all code, refs and history.
That single fact is why 56 gitmirror CR names are frozen with a stale
`-alice-<5char>` stem and why the fleet Kustomization runs prune:false
forever. A name should not be un-editable because the operator has no way
to say "release this CR, keep the thing".

Almost none of this is new: libseanfarm-operator's reconcile harness
already implements DeletionPolicy (Retain|Delete) and already skips the
remote delete for Retain, via the optional DeletionPolicyProvider
interface on the OBJECT. openbao and minio already implement it. The
Forgejo types simply never exposed the field, and objects that do not
implement the interface are treated as always-Delete.

Defaults follow what the remote state IS, not the kind's name. Retain for
the irreplaceable: repository (code + history) and team (membership and
permissions); also user and organization, whose Delete already returned
nil, so the field just makes existing behaviour legible. Delete for
grants, credentials and config -- a collaborator grant or access token
that outlives its CR is a security residue, so uniform Retain would be
the less safe choice.

Each accessor re-encodes its default for the empty string as well as via
+kubebuilder:default. Both are required: kubebuilder defaults apply at
WRITE time, so the 56 CRs already stored read back empty and would
otherwise fall through to the harness default of Delete -- destroying
precisely the state this field exists to protect.

The coverage test derives the kind SET from the scheme rather than a
hand-written list, so a CRD added later cannot inherit always-Delete
silently; the per-kind defaults stay a hand-written table because that
table is the spec. Mutation-tested both ways: flipping the repository
default to Delete and removing an accessor entirely are each caught.
sean merged commit bd871feafa into trunk 2026-08-07 08:47:29 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
sean/forgejo-operator!109
No description provided.