feat(chart): deny EtcdMember deletion at admission

charts/etcd-operator/templates/member-deletion-policy.yaml

@@ -0,0 +1,74 @@
+{{- /*
+Enforces at the API boundary the contract the docs have carried since day one:
+EtcdMember objects are created and deleted by the operator, not by users.
+
+Deleting one is not a recoverable mistake. Its data PVC is controller-owned by
+the member, so the volume — and on a Delete-reclaim StorageClass the data
+itself — goes with it, while the member's finalizer removes the member from
+etcd on the way out. A cluster whose members are deleted one by one therefore
+dismembers itself and leaves nothing to restore from.
+
+Denied here rather than survived in the controllers: the accident (a stray
+kubectl delete, a GitOps prune, a cleanup script sweeping CRs) is what needs to
+stop, and the requester learns why at the moment they try it.
+
+Allowed through:
+  - the operator's own ServiceAccount — scale-down and crash-loop replacement
+    delete members deliberately;
+  - the garbage collector — cascade from a deleted EtcdCluster must still work;
+  - the namespace controller — deleting a namespace must not hang.
+
+Break-glass without uninstalling the policy: annotate the member with
+etcd-operator.cozystack.io/allow-deletion=true, then delete it.
+
+Requires Kubernetes 1.30+ (ValidatingAdmissionPolicy GA). On an older
+apiserver the install fails with "no matches for kind
+ValidatingAdmissionPolicy"; set memberDeletionProtection.enabled=false to
+install without the guard. Deliberately not gated on .Capabilities: that is
+empty for a plain `helm template`, so gating there would silently drop the
+policy from the rendered release manifests.
+*/ -}}
+{{- if .Values.memberDeletionProtection.enabled }}
+{{- $allowed := concat
+      (list
+        (printf "system:serviceaccount:%s:%s" .Release.Namespace (include "etcd-operator.serviceAccountName" .))
+        "system:serviceaccount:kube-system:generic-garbage-collector"
+        "system:serviceaccount:kube-system:namespace-controller")
+      .Values.memberDeletionProtection.additionalAllowedUsers }}
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicy
+metadata:
+  name: {{ include "etcd-operator.fullname" . }}-protect-members
+  labels:
+    {{- include "etcd-operator.labels" . | nindent 4 }}
+spec:
+  failurePolicy: Fail
+  matchConstraints:
+    resourceRules:
+      - apiGroups: ["etcd-operator.cozystack.io"]
+        apiVersions: ["*"]
+        operations: ["DELETE"]
+        resources: ["etcdmembers"]
+  validations:
+    - expression: >-
+        request.userInfo.username in {{ $allowed | toJson }}
+        || (has(oldObject.metadata.annotations)
+            && "etcd-operator.cozystack.io/allow-deletion" in oldObject.metadata.annotations
+            && oldObject.metadata.annotations["etcd-operator.cozystack.io/allow-deletion"] == "true")
+      reason: Forbidden
+      messageExpression: >-
+        "EtcdMember " + oldObject.metadata.name + " is managed exclusively by etcd-operator, and deleting it
+        removes the member from etcd and releases its data volume — the data is not recoverable from here.
+        Scale the EtcdCluster instead. If you really mean it, annotate the member with
+        etcd-operator.cozystack.io/allow-deletion=true first."
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicyBinding
+metadata:
+  name: {{ include "etcd-operator.fullname" . }}-protect-members
+  labels:
+    {{- include "etcd-operator.labels" . | nindent 4 }}
+spec:
+  policyName: {{ include "etcd-operator.fullname" . }}-protect-members
+  validationActions: ["Deny"]
+{{- end }}

charts/etcd-operator/tests/member_deletion_policy_test.yaml

@@ -0,0 +1,128 @@
+suite: EtcdMember deletion is denied at admission
+
+# The policy is the enforcement point for a contract the docs have always
+# stated: EtcdMember objects belong to the operator. What matters in these
+# assertions is not the YAML shape but who keeps the ability to delete —
+# getting that list wrong either wedges cluster/namespace deletion (too
+# strict) or leaves the hole open (too loose).
+
+templates:
+  - member-deletion-policy.yaml
+
+tests:
+  - it: is installed by default
+    asserts:
+      - hasDocuments:
+          count: 2
+      - containsDocument:
+          apiVersion: admissionregistration.k8s.io/v1
+          kind: ValidatingAdmissionPolicy
+          name: RELEASE-NAME-etcd-operator-protect-members
+        documentIndex: 0
+      - containsDocument:
+          apiVersion: admissionregistration.k8s.io/v1
+          kind: ValidatingAdmissionPolicyBinding
+          name: RELEASE-NAME-etcd-operator-protect-members
+        documentIndex: 1
+      # The binding must name the policy it enforces — a typo here installs a
+      # policy that matches nothing and silently protects nothing.
+      - equal:
+          path: spec.policyName
+          value: RELEASE-NAME-etcd-operator-protect-members
+        documentIndex: 1
+
+  - it: matches only DELETE on etcdmembers
+    documentSelector:
+      path: kind
+      value: ValidatingAdmissionPolicy
+    asserts:
+      - equal:
+          path: spec.matchConstraints.resourceRules[0].operations
+          value: ["DELETE"]
+      - equal:
+          path: spec.matchConstraints.resourceRules[0].resources
+          value: ["etcdmembers"]
+      - equal:
+          path: spec.matchConstraints.resourceRules[0].apiGroups
+          value: ["etcd-operator.cozystack.io"]
+
+  - it: denies rather than warns, and fails closed
+    asserts:
+      - equal:
+          path: spec.validationActions
+          value: ["Deny"]
+        documentSelector:
+          path: kind
+          value: ValidatingAdmissionPolicyBinding
+      - equal:
+          path: spec.failurePolicy
+          value: Fail
+        documentSelector:
+          path: kind
+          value: ValidatingAdmissionPolicy
+
+  # The operator deletes members itself on scale-down and crash-loop
+  # replacement; the GC has to cascade from a deleted EtcdCluster; the
+  # namespace controller has to finish a namespace deletion. Denying any of
+  # the three turns a routine operation into a wedge.
+  - it: still allows the operator, the garbage collector and the namespace controller
+    documentSelector:
+      path: kind
+      value: ValidatingAdmissionPolicy
+    asserts:
+      - matchRegex:
+          path: spec.validations[0].expression
+          pattern: system:serviceaccount:NAMESPACE:RELEASE-NAME-etcd-operator
+      - matchRegex:
+          path: spec.validations[0].expression
+          pattern: system:serviceaccount:kube-system:generic-garbage-collector
+      - matchRegex:
+          path: spec.validations[0].expression
+          pattern: system:serviceaccount:kube-system:namespace-controller
+
+  - it: honours the break-glass annotation
+    documentSelector:
+      path: kind
+      value: ValidatingAdmissionPolicy
+    asserts:
+      - matchRegex:
+          path: spec.validations[0].expression
+          pattern: etcd-operator\.cozystack\.io/allow-deletion
+
+  # A platform whose own controller legitimately reaps members needs a way in
+  # that does not mean disabling the guard outright.
+  - it: accepts additional allowed users
+    set:
+      memberDeletionProtection:
+        additionalAllowedUsers:
+          - system:serviceaccount:platform:reaper
+    documentSelector:
+      path: kind
+      value: ValidatingAdmissionPolicy
+    asserts:
+      - matchRegex:
+          path: spec.validations[0].expression
+          pattern: system:serviceaccount:platform:reaper
+
+  # Kubernetes below 1.30 has no ValidatingAdmissionPolicy; the chart must
+  # still install there.
+  - it: can be switched off for older apiservers
+    set:
+      memberDeletionProtection:
+        enabled: false
+    asserts:
+      - hasDocuments:
+          count: 0
+
+  - it: names the ServiceAccount the release actually uses
+    set:
+      serviceAccount:
+        create: false
+        name: byo-sa
+    documentSelector:
+      path: kind
+      value: ValidatingAdmissionPolicy
+    asserts:
+      - matchRegex:
+          path: spec.validations[0].expression
+          pattern: system:serviceaccount:NAMESPACE:byo-sa

charts/etcd-operator/values.yaml

@@ -9,6 +9,28 @@ crds:
   # EtcdCluster and its data.
   keep: true
 
+# Deny DELETE on EtcdMember objects at the API boundary, except for the
+# operator itself, the garbage collector (so a deleted EtcdCluster still
+# cascades) and the namespace controller (so namespace deletion does not hang).
+#
+# Deleting an EtcdMember by hand is not a recoverable mistake: its data PVC is
+# controller-owned by the member, so the volume goes with it, while the
+# member's finalizer removes the member from etcd on the way out. The docs have
+# always said these objects are the operator's; this enforces it.
+#
+# Break-glass without uninstalling: annotate the member with
+# etcd-operator.cozystack.io/allow-deletion=true, then delete it.
+#
+# Requires Kubernetes 1.30+ (ValidatingAdmissionPolicy GA). Set to false on
+# older apiservers.
+memberDeletionProtection:
+  # -- Install the ValidatingAdmissionPolicy protecting EtcdMember objects.
+  enabled: true
+  # -- Extra usernames allowed to delete EtcdMembers, in apiserver form
+  # (e.g. "system:serviceaccount:<ns>:<sa>" or a user name). For platforms
+  # whose own controllers legitimately reap these objects.
+  additionalAllowedUsers: []
+
 # -- Render a Namespace object. Off by default (real `helm install` uses
 # --create-namespace); build-dist-manifests turns it on so the rendered
 # kubectl-apply manifest is self-contained.

docs/concepts.md

@@ -10,12 +10,37 @@ Two custom resources, one of them user-facing.
 
 **`EtcdCluster`** — the user-facing object. It captures cluster-wide intent: replica count, etcd version, per-member storage size, a progress deadline. This is the only resource users normally touch.
 
-**`EtcdMember`** — one per etcd member. Created and deleted by the cluster controller. Each `EtcdMember` owns its Pod and PVC. Users should not create or edit these directly.
+**`EtcdMember`** — one per etcd member. Created and deleted by the cluster controller. Each `EtcdMember` owns its Pod and PVC. Users should not create, edit or **delete** these directly.
+
+Deleting one by hand is not a recoverable mistake: the member's PVC is controller-owned by it, so the data volume is removed with the CR (and on a `Delete`-reclaim StorageClass the data itself), while the member's finalizer removes the member from etcd on the way out. Delete every member of a cluster and it dismembers itself, leaving nothing to restore from. Scale the `EtcdCluster` instead — see [member deletion is denied at admission](#member-deletion-is-denied-at-admission).
 
 There is **no StatefulSet**. Each member's Pod and PVC are reconciled independently by the member controller. The motivation is protocol awareness: scale-up adds a member as a learner first and only promotes once it's caught up; scale-down runs `MemberRemove` via a finalizer before reclaiming the Pod; pod restarts reuse the existing data dir and rejoin with the same etcd-side member ID. None of these flows fit StatefulSet's "all replicas are one fungible workload" model.
 
 The cluster controller decides *which* members exist and orchestrates the etcd-side state machine (`MemberAddAsLearner` / `MemberPromote` / `MemberRemove`). The member controller decides *how* a member becomes real — Pod, PVC, etcd flags — and reports observed facts (member ID, readiness) back up to its CR's status.
 
+## Member deletion is denied at admission
+
+The chart installs a `ValidatingAdmissionPolicy` that rejects `DELETE` on `etcdmembers` for everyone except:
+
+- the operator's own ServiceAccount — scale-down and crash-loop replacement delete members deliberately;
+- `system:serviceaccount:kube-system:generic-garbage-collector` — a deleted `EtcdCluster` must still cascade to its members;
+- `system:serviceaccount:kube-system:namespace-controller` — deleting a namespace must not hang;
+- anything listed in `memberDeletionProtection.additionalAllowedUsers`, for platforms whose own controllers legitimately reap these objects.
+
+The rejection message says what would have happened and how to proceed deliberately. Break-glass without uninstalling the policy:
+
+```sh
+kubectl annotate etcdmember.etcd-operator.cozystack.io <member> -n <ns> \
+  etcd-operator.cozystack.io/allow-deletion=true
+kubectl delete etcdmember.etcd-operator.cozystack.io <member> -n <ns>
+```
+
+The guard exists because the accident it prevents is unrecoverable and easy: one `kubectl delete`, a GitOps prune of an unexpected object, a cleanup script sweeping CRs by label. The controllers cannot make that survivable — a member's data volume is bound to its identity, and a replacement member gets a fresh name and UID — so the event is stopped at the boundary instead.
+
+Requires Kubernetes 1.30+ (`ValidatingAdmissionPolicy` GA). On older apiservers, install with `memberDeletionProtection.enabled=false`; the operator behaves as before, without the guard.
+
+**Uninstalling:** remove the policy before removing the CRDs. `helm uninstall` does this in the right order, but a manual teardown that deletes the CRDs first will find member deletion denied and stall.
+
 ## Member naming
 
 `EtcdMember` CRs are created with `ObjectMeta.GenerateName="<cluster>-"`. Each member's name is an apiserver-assigned random suffix (e.g. `mycluster-7xq2k`). Names are not predictable, and that is deliberate — the previous design used `<cluster>-<ordinal>` and tied cluster identity to ordinal reuse across incarnations, which is exactly the trap to avoid for stateful systems. Now:

docs/installation.md

@@ -311,6 +311,13 @@ kubectl delete etcdcluster.etcd-operator.cozystack.io --all -A
 # intentionally left in place.
 make undeploy
 
+# Remove the member-deletion guard before the CRDs. Removing the CRD makes the
+# apiserver delete every EtcdMember, and the policy would deny those requests —
+# leaving the CRD stuck in Terminating. `helm uninstall` above already removed
+# it; this is for teardowns that skipped Helm:
+kubectl delete validatingadmissionpolicybinding etcd-operator-protect-members --ignore-not-found
+kubectl delete validatingadmissionpolicy etcd-operator-protect-members --ignore-not-found
+
 # Remove the CRDs too (only after all EtcdClusters are gone) — deleting them
 # cascade-deletes every remaining EtcdCluster:
 kubectl delete crd etcdclusters.etcd-operator.cozystack.io \

docs/operations.md

@@ -214,14 +214,20 @@ Manual recovery:
 ```sh
 # 1. Identify the broken member.
 kubectl get etcdmember.etcd-operator.cozystack.io -n <ns>
-# 2. Delete it — the finalizer runs MemberRemove against peers, then GC takes
+# 2. Deleting a member is denied by default (see concepts: member deletion is
+#    denied at admission) — this is the deliberate exception, so unlock it:
+kubectl annotate etcdmember.etcd-operator.cozystack.io <broken-member> -n <ns> \
+  etcd-operator.cozystack.io/allow-deletion=true
+# 3. Delete it — the finalizer runs MemberRemove against peers, then GC takes
 #    the Pod and PVC. Quorum holds because we remove before adding.
 kubectl delete etcdmember.etcd-operator.cozystack.io <broken-member> -n <ns>
-# 3. The cluster controller's next reconcile observes current < desired and
+# 4. The cluster controller's next reconcile observes current < desired and
 #    scales up automatically — a new member is added with GenerateName and
 #    fresh storage.
 ```
 
+The annotation step is the point of the guard: this recovery discards a data volume on purpose, and typing that out is what separates it from the same command issued by accident.
+
 This sequence preserves quorum if you have an odd number of voters and only one is broken. If multiple voters are broken simultaneously, quorum is lost and you can't `MemberRemove` cleanly. In that case the recovery is to delete the EtcdCluster, recreate it, and restore from a snapshot — see [Restoring a cluster from a snapshot](#restoring-a-cluster-from-a-snapshot). Snapshots only exist if you have been taking `EtcdSnapshot`s, so set that up *before* you need it.
 
 ## Taking a snapshot

test/e2e/kamaji_datastore_test.go

@@ -165,7 +165,22 @@ func TestKamajiDataStore(t *testing.T) {
 	t.Logf("original members: %v", original)
 	for _, victim := range original {
 		t.Logf("deleting EtcdMember %q (operator does MemberRemove + a GenerateName replacement)", victim)
-		m := &etcdv1alpha2.EtcdMember{ObjectMeta: metav1.ObjectMeta{Namespace: e2eNamespace, Name: victim}}
+		// Member deletion is denied at admission — the guard exists precisely to
+		// stop this command when it is issued by accident. This test issues it on
+		// purpose, so it takes the documented break-glass path: annotate first.
+		// Doing it here rather than exempting the test's identity in the policy
+		// keeps the e2e honest about what a human has to do.
+		m := &etcdv1alpha2.EtcdMember{}
+		if err := kube.Get(ctx, client.ObjectKey{Namespace: e2eNamespace, Name: victim}, m); err != nil {
+			t.Fatalf("get member %s before deletion: %v", victim, err)
+		}
+		if m.Annotations == nil {
+			m.Annotations = map[string]string{}
+		}
+		m.Annotations["etcd-operator.cozystack.io/allow-deletion"] = "true"
+		if err := kube.Update(ctx, m); err != nil {
+			t.Fatalf("annotate member %s for deletion: %v", victim, err)
+		}
 		if err := kube.Delete(ctx, m); err != nil && !apierrors.IsNotFound(err) {
 			t.Fatalf("delete member %s: %v", victim, err)
 		}