feat(chart): deny EtcdMember deletion at admission

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

@@ -0,0 +1,99 @@
+{{- /*
+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.
+
+Nothing manages EtcdMember objects declaratively (the only sanctioned
+non-operator writer is cmd/etcd-migrate, which creates and never deletes), so a
+DELETE from anywhere else is always an accident or a tracking misconfiguration
+— stopped at the boundary, with the requester told why.
+
+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;
+  - kube-controller-manager itself — on clusters run without
+    --use-service-account-credentials the GC and namespace-cleanup deletes
+    authenticate as the user system:kube-controller-manager rather than the
+    per-controller ServiceAccounts above.
+
+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.
+
+Rendered unconditionally (NOT gated on .Capabilities.APIVersions): capability
+detection would make a security guard's absence silent — a pre-1.30 `helm
+install`, or a GitOps render carrying the destination's API versions, would
+quietly omit it. Absence of a guard against an unrecoverable accident must be
+loud and explicit: always render, and opt out via
+memberDeletionProtection.enabled=false.
+*/ -}}
+{{- 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"
+        "system:kube-controller-manager")
+      .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"]
+{{- with .Values.manager.watchNamespaces }}
+  # Scope to the watched namespaces so multiple namespace-scoped releases don't
+  # deny each other's members (admission is deny-wins; each allowlist holds only
+  # its own operator SA). Cluster-wide install must be a singleton — see values.yaml.
+  matchResources:
+    namespaceSelector:
+      matchExpressions:
+        - key: kubernetes.io/metadata.name
+          operator: In
+          values:
+            {{- range . }}
+            - {{ . | quote }}
+            {{- end }}
+{{- end }}
+{{- end }}

charts/etcd-operator/tests/member_deletion_policy_test.yaml

@@ -0,0 +1,165 @@
+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, the namespace controller and kube-controller-manager
+    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
+      # Clusters without --use-service-account-credentials run GC and namespace
+      # cleanup as the user system:kube-controller-manager, not per-controller SAs.
+      - matchRegex:
+          path: spec.validations[0].expression
+          pattern: system:kube-controller-manager
+
+  - 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
+
+  # Cluster-wide by default (no watchNamespaces): the binding must match every
+  # namespace, so it carries no matchResources scoping.
+  - it: binds cluster-wide when the operator watches all namespaces
+    documentSelector:
+      path: kind
+      value: ValidatingAdmissionPolicyBinding
+    asserts:
+      - notExists:
+          path: spec.matchResources
+
+  # Namespace-scoped operator: the binding must scope to the watched namespaces
+  # so two scoped releases don't deny each other's members.
+  - it: scopes the binding to the watched namespaces when set
+    set:
+      manager:
+        watchNamespaces:
+          - team-a
+          - team-b
+    documentSelector:
+      path: kind
+      value: ValidatingAdmissionPolicyBinding
+    asserts:
+      - equal:
+          path: spec.matchResources.namespaceSelector.matchExpressions[0].key
+          value: kubernetes.io/metadata.name
+      - equal:
+          path: spec.matchResources.namespaceSelector.matchExpressions[0].operator
+          value: In
+      - equal:
+          path: spec.matchResources.namespaceSelector.matchExpressions[0].values
+          value: [team-a, team-b]

charts/etcd-operator/values.yaml

@@ -9,6 +9,33 @@ 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.
+#
+# Scope: with manager.watchNamespaces set, the binding is scoped to those
+# namespaces so scoped releases coexist. Cluster-wide (watchNamespaces empty)
+# the install must be a singleton — a second cluster-wide release would deny
+# this operator's own scale-down/replacement (admission is deny-wins).
+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,39 @@ 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, and because nothing legitimately manages `EtcdMember` objects declaratively — the only sanctioned non-operator writer is `cmd/etcd-migrate`, which creates and never deletes. So a DELETE from anywhere else — a stray `kubectl delete`, a cleanup script sweeping CRs by label, a GitOps tool that wrongly tracks these objects (a tracking misconfiguration, not a workflow to support) — is always an accident. The controllers cannot make it survivable (a member's data volume is bound to its identity, and a replacement 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.
+
+**Known limitation:** the guard does not cover member removal driven by *CRD* deletion. When the `etcdmembers` CRD is deleted, apiextensions cleans up the CR instances in-process, through the storage layer rather than the authenticated request path — the same route that keeps admission webhooks from firing for CRs deleted during CRD deletion — so admission (and this policy) never sees those deletes. `crds.keep=true` (the default) is what actually protects against that path.
+
+**Uninstalling:** `helm uninstall` removes the policy for you. A manual teardown should remove the policy too (see the [teardown runbook](installation.md#teardown)); ordering it before the CRD deletion is tidy but not load-bearing — per the limitation above, CRD cleanup bypasses the policy rather than stalling on it.
+
 ## 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,17 @@ kubectl delete etcdcluster.etcd-operator.cozystack.io --all -A
 # intentionally left in place.
 make undeploy
 
+# Remove the member-deletion guard. `helm uninstall` above already removed it;
+# this is for teardowns that skipped Helm. Order does not actually matter for
+# the CRD: apiextensions cleans up CR instances in-process during CRD deletion,
+# which bypasses admission, so the policy neither denies those deletes nor
+# stalls the CRD (this is also why CRD-deletion-driven member removal is a
+# known gap the guard does not cover — see concepts.md). Delete by label so it
+# works regardless of the release name (add app.kubernetes.io/instance=<release>
+# to disambiguate when several releases are installed):
+kubectl delete validatingadmissionpolicybinding,validatingadmissionpolicy \
+  -l app.kubernetes.io/name=etcd-operator --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