diff --git a/charts/etcd-operator/templates/member-deletion-policy.yaml b/charts/etcd-operator/templates/member-deletion-policy.yaml new file mode 100644 index 00000000..03a99161 --- /dev/null +++ b/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 }} diff --git a/charts/etcd-operator/tests/member_deletion_policy_test.yaml b/charts/etcd-operator/tests/member_deletion_policy_test.yaml new file mode 100644 index 00000000..b82f0b5d --- /dev/null +++ b/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 diff --git a/charts/etcd-operator/values.yaml b/charts/etcd-operator/values.yaml index 04233fbb..3311feae 100644 --- a/charts/etcd-operator/values.yaml +++ b/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::" 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. diff --git a/docs/concepts.md b/docs/concepts.md index 6cd535ba..207dddcd 100644 --- a/docs/concepts.md +++ b/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 -n \ + etcd-operator.cozystack.io/allow-deletion=true +kubectl delete etcdmember.etcd-operator.cozystack.io -n +``` + +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="-"`. 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 `-` and tied cluster identity to ordinal reuse across incarnations, which is exactly the trap to avoid for stateful systems. Now: diff --git a/docs/installation.md b/docs/installation.md index d8c6ab4c..c7d00250 100644 --- a/docs/installation.md +++ b/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 \ diff --git a/docs/operations.md b/docs/operations.md index 48649db8..72262c0e 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -214,14 +214,20 @@ Manual recovery: ```sh # 1. Identify the broken member. kubectl get etcdmember.etcd-operator.cozystack.io -n -# 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 -n \ + 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 -n -# 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 diff --git a/test/e2e/kamaji_datastore_test.go b/test/e2e/kamaji_datastore_test.go index a93bd9de..790cb2a9 100644 --- a/test/e2e/kamaji_datastore_test.go +++ b/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) }