diff --git a/README.md b/README.md index 64669805..98531b3f 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ The full design rationale is in [docs/concepts.md](docs/concepts.md). - **Memory-backed storage (opt-in)**: `spec.storage.medium: Memory` switches each member's data dir to a tmpfs `emptyDir` whose lifetime is bound to the Pod. Members that lose their Pod (eviction, node failure) lose their data; the operator detects this, removes the member from etcd, and replaces it via the existing scale-up path. Suits scenarios where the etcd state is reconstructable and replication absorbs single-member losses. For production, set `spec.affinity` and `spec.resources.limits.memory` explicitly — neither is defaulted ([#16](https://github.com/lllamnyp/etcd-operator/issues/16)); see [docs/concepts.md](docs/concepts.md#storage). - **Apiserver-enforced validation**: CEL rules on the CRD (k8s 1.29+) reject `replicas: 0` with `storage.medium: Memory`, `storage.size: 0` with `storage.medium: Memory`, `storage.medium` changes after creation, and `storage.size` shrinks. No webhook / cert-manager dependency. - **PodDisruptionBudget**: per-cluster PDB selects voting members only (`role=voter`); `maxUnavailable = (voters-1)/2` so `kubectl drain` cannot voluntarily push the cluster below quorum. -- **TLS (BYO Secrets or cert-manager)**: `spec.tls.client` / `spec.tls.peer` enable TLS on each surface independently. Material comes from either user-provided Secrets (`serverSecretRef` / `operatorClientSecretRef` / `secretRef`) or operator-emitted `cert-manager.io/v1` Certificates (`certManager.{serverIssuerRef,operatorClientIssuerRef,issuerRef}`) — mutually exclusive per subtree, enforced by CEL. mTLS is the implicit mode when an operator-client source is supplied; server-TLS-only when it isn't. The whole `tls` subtree is CEL-locked immutable post-create. cert-manager-emitted certs auto-renew via cert-manager; Pod-side rotation is a manual one-at-a-time `kubectl delete pod` either way. See [docs/concepts.md](docs/concepts.md#tls). +- **TLS (BYO Secrets or cert-manager)**: `spec.tls.client` / `spec.tls.peer` enable TLS on each surface independently. Material comes from either user-provided Secrets (`serverSecretRef` / `operatorClientSecretRef` / `secretRef`) or operator-emitted `cert-manager.io/v1` Certificates (`certManager.{serverIssuerRef,operatorClientIssuerRef,issuerRef}`) — mutually exclusive per subtree, enforced by CEL. mTLS is the implicit mode when an operator-client source is supplied; server-TLS-only when it isn't. The `tls` subtree is CEL-locked immutable post-create except for one move: handing the material over from user-provided Secrets to operator-managed cert-manager issuance, which the operator performs itself — see [Handing TLS over to the operator](docs/operations.md#handing-tls-over-to-the-operator). cert-manager-emitted certs auto-renew via cert-manager; Pod-side rotation is a manual one-at-a-time `kubectl delete pod` either way. See [docs/concepts.md](docs/concepts.md#tls). - **Resource sizing**: `spec.resources` (a `corev1.ResourceRequirements`) sets the etcd container's CPU/memory requests and limits. Unset uses a conservative 100m/128Mi-request default. Updates take effect on newly-created members; pair with a `VerticalPodAutoscaler` targeting the cluster for live recommendation/rollout. - **Scheduling & extra metadata**: `spec.affinity` and `spec.topologySpreadConstraints` pass through to every member Pod (anti-affinity is not defaulted — set it for production); `spec.additionalMetadata` merges user labels/annotations onto every object the operator creates (member Pods, data PVCs, Services, PDB, `EtcdMember` CRs), with operator-owned keys winning on collision. All three apply on object creation and are latched like the rest of the spec. See [docs/concepts.md](docs/concepts.md#pod-scheduling-and-additional-metadata). - **Monitoring / autoscaling hooks**: every member Pod always exposes a plaintext `metrics` container port at `2381` (etcd's `/health` + Prometheus `/metrics`) for `VMPodScrape` / `PodMonitor`. The `EtcdCluster` CRD exposes the `/scale` subresource with a populated `status.selector`, making it a valid target for `kubectl scale` and `VerticalPodAutoscaler.targetRef`. diff --git a/api/v1alpha2/cel_validation_test.go b/api/v1alpha2/cel_validation_test.go index 0ed601c8..ca964344 100644 --- a/api/v1alpha2/cel_validation_test.go +++ b/api/v1alpha2/cel_validation_test.go @@ -292,11 +292,195 @@ func TestCEL_TLSSubfieldChangeRejected(t *testing.T) { if err == nil { t.Fatalf("apiserver accepted mTLS toggle (added operatorClientSecretRef); expected rejection") } - if !strings.Contains(err.Error(), "spec.tls is immutable") { + if !strings.Contains(err.Error(), "spec.tls.client is immutable") { t.Fatalf("error did not mention subtree immutability: %v", err) } } +// The one permitted spec.tls change: handing the material over from +// user-provided Secrets to operator-managed cert-manager issuance, on both +// planes at once. +func TestCEL_TLSHandoverToCertManagerAccepted(t *testing.T) { + skipIfNoEnvtest(t) + ctx := context.Background() + + c := validCluster("tls-handover-ok") + c.Spec.TLS = &lll.EtcdClusterTLS{ + Client: &lll.ClientTLS{ + ServerSecretRef: &corev1.LocalObjectReference{Name: "chart-server-tls"}, + }, + Peer: &lll.PeerTLS{ + SecretRef: &corev1.LocalObjectReference{Name: "chart-peer-tls"}, + }, + } + if err := k8s.Create(ctx, c); err != nil { + t.Fatalf("Create BYO-TLS cluster: %v", err) + } + t.Cleanup(func() { _ = k8s.Delete(ctx, c) }) + + got := &lll.EtcdCluster{} + if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(c), got); err != nil { + t.Fatalf("Get: %v", err) + } + got.Spec.TLS.Client = &lll.ClientTLS{ + CertManager: &lll.ClientCertManagerTLS{ + ServerIssuerRef: lll.IssuerReference{Name: "etcd-issuer"}, + }, + } + got.Spec.TLS.Peer = &lll.PeerTLS{ + CertManager: &lll.PeerCertManagerTLS{ + IssuerRef: lll.IssuerReference{Name: "etcd-peer-issuer"}, + }, + } + if err := k8s.Update(ctx, got); err != nil { + t.Fatalf("apiserver rejected the BYO->cert-manager handover: %v", err) + } +} + +// The handover is one-way. Going back to BYO Secrets would strand the +// cluster on material the operator is about to GC. +func TestCEL_TLSHandoverBackToSecretRefRejected(t *testing.T) { + skipIfNoEnvtest(t) + ctx := context.Background() + + c := validCluster("tls-handover-reverse") + c.Spec.TLS = &lll.EtcdClusterTLS{ + Peer: &lll.PeerTLS{ + CertManager: &lll.PeerCertManagerTLS{ + IssuerRef: lll.IssuerReference{Name: "etcd-peer-issuer"}, + }, + }, + } + if err := k8s.Create(ctx, c); err != nil { + t.Fatalf("Create cert-manager cluster: %v", err) + } + t.Cleanup(func() { _ = k8s.Delete(ctx, c) }) + + got := &lll.EtcdCluster{} + if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(c), got); err != nil { + t.Fatalf("Get: %v", err) + } + got.Spec.TLS.Peer = &lll.PeerTLS{ + SecretRef: &corev1.LocalObjectReference{Name: "chart-peer-tls"}, + } + + err := k8s.Update(ctx, got) + if err == nil { + t.Fatalf("apiserver accepted the reverse handover back to secretRef; expected rejection") + } + if !strings.Contains(err.Error(), "spec.tls.peer is immutable") { + t.Fatalf("error did not mention peer subtree immutability: %v", err) + } +} + +// The handover must not be a vehicle for silently dropping client mTLS: +// a cluster that presented an operator client cert has to keep presenting +// one on the other side of the move. +func TestCEL_TLSHandoverDroppingClientMTLSRejected(t *testing.T) { + skipIfNoEnvtest(t) + ctx := context.Background() + + c := validCluster("tls-handover-mtls") + c.Spec.TLS = &lll.EtcdClusterTLS{ + Client: &lll.ClientTLS{ + ServerSecretRef: &corev1.LocalObjectReference{Name: "chart-server-tls"}, + OperatorClientSecretRef: &corev1.LocalObjectReference{Name: "chart-client-tls"}, + }, + } + if err := k8s.Create(ctx, c); err != nil { + t.Fatalf("Create mTLS cluster: %v", err) + } + t.Cleanup(func() { _ = k8s.Delete(ctx, c) }) + + got := &lll.EtcdCluster{} + if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(c), got); err != nil { + t.Fatalf("Get: %v", err) + } + // certManager without operatorClientIssuerRef == server-TLS only. + got.Spec.TLS.Client = &lll.ClientTLS{ + CertManager: &lll.ClientCertManagerTLS{ + ServerIssuerRef: lll.IssuerReference{Name: "etcd-issuer"}, + }, + } + + err := k8s.Update(ctx, got) + if err == nil { + t.Fatalf("apiserver accepted a handover that silently disabled client mTLS; expected rejection") + } + if !strings.Contains(err.Error(), "mTLS posture") { + t.Fatalf("error did not mention the mTLS posture requirement: %v", err) + } +} + +// The mTLS-preserving form of the same handover is accepted. +func TestCEL_TLSHandoverPreservingClientMTLSAccepted(t *testing.T) { + skipIfNoEnvtest(t) + ctx := context.Background() + + c := validCluster("tls-handover-mtls-ok") + c.Spec.TLS = &lll.EtcdClusterTLS{ + Client: &lll.ClientTLS{ + ServerSecretRef: &corev1.LocalObjectReference{Name: "chart-server-tls"}, + OperatorClientSecretRef: &corev1.LocalObjectReference{Name: "chart-client-tls"}, + }, + } + if err := k8s.Create(ctx, c); err != nil { + t.Fatalf("Create mTLS cluster: %v", err) + } + t.Cleanup(func() { _ = k8s.Delete(ctx, c) }) + + got := &lll.EtcdCluster{} + if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(c), got); err != nil { + t.Fatalf("Get: %v", err) + } + got.Spec.TLS.Client = &lll.ClientTLS{ + CertManager: &lll.ClientCertManagerTLS{ + ServerIssuerRef: lll.IssuerReference{Name: "etcd-issuer"}, + OperatorClientIssuerRef: &lll.IssuerReference{Name: "etcd-issuer"}, + }, + } + if err := k8s.Update(ctx, got); err != nil { + t.Fatalf("apiserver rejected an mTLS-preserving handover: %v", err) + } +} + +// The TLS subtrees themselves still cannot appear or vanish — the handover +// exception is about the source of the material, not about turning a plane +// on or off. +func TestCEL_TLSPeerSubtreeAddRejected(t *testing.T) { + skipIfNoEnvtest(t) + ctx := context.Background() + + c := validCluster("tls-peer-add") + c.Spec.TLS = &lll.EtcdClusterTLS{ + Client: &lll.ClientTLS{ + ServerSecretRef: &corev1.LocalObjectReference{Name: "chart-server-tls"}, + }, + } + if err := k8s.Create(ctx, c); err != nil { + t.Fatalf("Create client-only TLS cluster: %v", err) + } + t.Cleanup(func() { _ = k8s.Delete(ctx, c) }) + + got := &lll.EtcdCluster{} + if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(c), got); err != nil { + t.Fatalf("Get: %v", err) + } + got.Spec.TLS.Peer = &lll.PeerTLS{ + CertManager: &lll.PeerCertManagerTLS{ + IssuerRef: lll.IssuerReference{Name: "etcd-peer-issuer"}, + }, + } + + err := k8s.Update(ctx, got) + if err == nil { + t.Fatalf("apiserver accepted adding the peer TLS subtree post-create; expected rejection") + } + if !strings.Contains(err.Error(), "spec.tls.peer cannot be added") { + t.Fatalf("error did not mention peer subtree add/remove: %v", err) + } +} + // TestCEL_StorageClassNameAddRejected verifies that adding a // storageClassName after Create is rejected. A PVC's storageClassName // is itself immutable, so the operator can only honour a value chosen diff --git a/api/v1alpha2/etcdcluster_types.go b/api/v1alpha2/etcdcluster_types.go index 8bdd5253..b9145654 100644 --- a/api/v1alpha2/etcdcluster_types.go +++ b/api/v1alpha2/etcdcluster_types.go @@ -24,11 +24,24 @@ import ( // EtcdClusterTLS configures transport-layer security for the cluster's two // etcd surfaces: the client API (port 2379) and the peer API (port 2380). -// Each subtree is independently optional. Subtree fields are immutable -// post-create — flipping TLS on or off on an existing cluster is a -// non-trivial rolling change (the operator's own etcd client must switch -// protocols in lockstep with the members), so v1 punts that to delete-and- -// recreate. +// Each subtree is independently optional and immutable post-create, with +// one exception: the source of the material may be handed over from +// user-provided Secrets to operator-managed cert-manager issuance +// (Client.ServerSecretRef → Client.CertManager, Peer.SecretRef → +// Peer.CertManager). The operator drives that handover itself — it issues +// the new material, waits for cert-manager to populate it, then rolls every +// member onto it at once. +// +// Nothing else about the subtree may change. Flipping TLS on or off, or +// handing back from cert-manager to BYO Secrets, still requires delete-and- +// recreate: the operator's own etcd client would have to switch protocols +// or trust roots in lockstep with the members, and there is no safe +// operator-driven sequence for that. +// +// The handover is deliberately one-way. Going back to BYO would mean +// trusting material the operator cannot verify is in place, and the +// cert-manager Certificates it owns would be GC'd out from under a running +// cluster the moment the spec stopped referencing them. type EtcdClusterTLS struct { // Client configures TLS for the etcd client API (port 2379). Absent // means plaintext. See ClientTLS for the mTLS-toggle semantics. @@ -307,6 +320,31 @@ const ( ClusterProgressing = "Progressing" // ClusterDegraded indicates some members are unhealthy but quorum holds. ClusterDegraded = "Degraded" + // ClusterTLSHandover tracks the one-way move from user-provided TLS + // Secrets to operator-managed cert-manager material. True while a + // handover is in flight; False once it has settled or when it is + // blocked. It is deliberately a type of its own rather than a reason + // on Available/Degraded/Progressing: updateStatus rewrites those three + // from quorum health on every pass, so a handover signal parked on any + // of them would be clobbered within seconds. + ClusterTLSHandover = "TLSHandover" +) + +// Condition reasons for ClusterTLSHandover. +const ( + // TLSHandoverAwaitingMaterial: the operator has requested the new + // Certificates and is waiting for cert-manager to populate the Secrets. + // No member has been touched yet. + TLSHandoverAwaitingMaterial = "AwaitingMaterial" + // TLSHandoverRollingMembers: every member has been repointed at the new + // material and their Pods are being rebuilt. + TLSHandoverRollingMembers = "RollingMembers" + // TLSHandoverBlocked: a piece of TLS material the operator must own is + // held by another controller. Requires human intervention; the cluster + // keeps running on the material it already has. + TLSHandoverBlocked = "Blocked" + // TLSHandoverComplete: every member runs on operator-managed material. + TLSHandoverComplete = "Complete" ) // BootstrapSpec configures one-time cluster initialization. Consulted only @@ -425,10 +463,25 @@ type StorageSpec struct { // the tmpfs is unbounded against node memory, which defeats the whole // point of opting into memory backing. // +// - spec.tls is immutable post-create with exactly one exception: the +// one-way handover from user-provided Secrets to operator-managed +// cert-manager issuance (client.serverSecretRef → client.certManager, +// peer.secretRef → peer.certManager). Everything else about the TLS +// subtree stays frozen — the plaintext↔TLS flip, the reverse handover +// back to BYO Secrets, swapping one Secret ref for another, and +// toggling the client-mTLS posture. The handover is permitted because +// the operator can drive it safely end to end (issue the new material, +// wait for it, then roll every member onto it in one step); the +// others have no such path and still require delete-and-recreate. +// Progress is reported on the TLSHandover status condition. +// // +kubebuilder:validation:XValidation:rule="!(has(self.replicas) && self.replicas == 0 && has(self.storage) && has(self.storage.medium) && self.storage.medium == 'Memory')",message="spec.replicas=0 with spec.storage.medium=Memory is unsupported: pausing a memory-backed cluster wedges on resume. Delete and recreate the cluster instead." // +kubebuilder:validation:XValidation:rule="!(has(self.storage) && has(self.storage.medium) && self.storage.medium == 'Memory') || quantity(string(self.storage.size)).isGreaterThan(quantity('0'))",message="spec.storage.size must be > 0 when spec.storage.medium=Memory (the tmpfs sizeLimit cannot be zero)." // +kubebuilder:validation:XValidation:rule="has(self.tls) == has(oldSelf.tls)",message="spec.tls cannot be added to or removed from an existing cluster; delete and recreate" -// +kubebuilder:validation:XValidation:rule="!has(self.tls) || !has(oldSelf.tls) || self.tls == oldSelf.tls",message="spec.tls is immutable post-create; delete and recreate the cluster to change TLS configuration" +// +kubebuilder:validation:XValidation:rule="!has(self.tls) || !has(oldSelf.tls) || has(self.tls.client) == has(oldSelf.tls.client)",message="spec.tls.client cannot be added to or removed from an existing cluster; delete and recreate" +// +kubebuilder:validation:XValidation:rule="!has(self.tls) || !has(oldSelf.tls) || has(self.tls.peer) == has(oldSelf.tls.peer)",message="spec.tls.peer cannot be added to or removed from an existing cluster; delete and recreate" +// +kubebuilder:validation:XValidation:rule="!has(self.tls) || !has(oldSelf.tls) || !has(self.tls.client) || self.tls.client == oldSelf.tls.client || (has(oldSelf.tls.client.serverSecretRef) && has(self.tls.client.certManager) && has(oldSelf.tls.client.operatorClientSecretRef) == has(self.tls.client.certManager.operatorClientIssuerRef))",message="spec.tls.client is immutable post-create except for the one-way handover from serverSecretRef to certManager, which must preserve the mTLS posture (operatorClientSecretRef set iff certManager.operatorClientIssuerRef is set)" +// +kubebuilder:validation:XValidation:rule="!has(self.tls) || !has(oldSelf.tls) || !has(self.tls.peer) || self.tls.peer == oldSelf.tls.peer || (has(oldSelf.tls.peer.secretRef) && has(self.tls.peer.certManager))",message="spec.tls.peer is immutable post-create except for the one-way handover from secretRef to certManager" // +kubebuilder:validation:XValidation:rule="has(self.storage.storageClassName) == has(oldSelf.storage.storageClassName)",message="spec.storage.storageClassName cannot be added to or removed from an existing cluster; delete and recreate" // +kubebuilder:validation:XValidation:rule="!has(self.storage.storageClassName) || !has(oldSelf.storage.storageClassName) || self.storage.storageClassName == oldSelf.storage.storageClassName",message="spec.storage.storageClassName is immutable post-create (a PVC's storageClassName itself is immutable, and the operator does not roll PVCs); delete and recreate the cluster to change the StorageClass" // +kubebuilder:validation:XValidation:rule="has(self.auth) == has(oldSelf.auth)",message="spec.auth cannot be added to or removed from an existing cluster; delete and recreate" diff --git a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml index ed4d78ad..b9e0950e 100644 --- a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml +++ b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml @@ -65,6 +65,18 @@ spec: - storage.medium=Memory requires storage.size > 0. Without a SizeLimit the tmpfs is unbounded against node memory, which defeats the whole point of opting into memory backing. + + - spec.tls is immutable post-create with exactly one exception: the + one-way handover from user-provided Secrets to operator-managed + cert-manager issuance (client.serverSecretRef → client.certManager, + peer.secretRef → peer.certManager). Everything else about the TLS + subtree stays frozen — the plaintext↔TLS flip, the reverse handover + back to BYO Secrets, swapping one Secret ref for another, and + toggling the client-mTLS posture. The handover is permitted because + the operator can drive it safely end to end (issue the new material, + wait for it, then roll every member onto it in one step); the + others have no such path and still require delete-and-recreate. + Progress is reported on the TLSHandover status condition. properties: additionalMetadata: description: |- @@ -1802,9 +1814,27 @@ spec: - message: spec.tls cannot be added to or removed from an existing cluster; delete and recreate rule: has(self.tls) == has(oldSelf.tls) - - message: spec.tls is immutable post-create; delete and recreate the - cluster to change TLS configuration - rule: '!has(self.tls) || !has(oldSelf.tls) || self.tls == oldSelf.tls' + - message: spec.tls.client cannot be added to or removed from an existing + cluster; delete and recreate + rule: '!has(self.tls) || !has(oldSelf.tls) || has(self.tls.client) == + has(oldSelf.tls.client)' + - message: spec.tls.peer cannot be added to or removed from an existing + cluster; delete and recreate + rule: '!has(self.tls) || !has(oldSelf.tls) || has(self.tls.peer) == + has(oldSelf.tls.peer)' + - message: spec.tls.client is immutable post-create except for the one-way + handover from serverSecretRef to certManager, which must preserve + the mTLS posture (operatorClientSecretRef set iff certManager.operatorClientIssuerRef + is set) + rule: '!has(self.tls) || !has(oldSelf.tls) || !has(self.tls.client) + || self.tls.client == oldSelf.tls.client || (has(oldSelf.tls.client.serverSecretRef) + && has(self.tls.client.certManager) && has(oldSelf.tls.client.operatorClientSecretRef) + == has(self.tls.client.certManager.operatorClientIssuerRef))' + - message: spec.tls.peer is immutable post-create except for the one-way + handover from secretRef to certManager + rule: '!has(self.tls) || !has(oldSelf.tls) || !has(self.tls.peer) || + self.tls.peer == oldSelf.tls.peer || (has(oldSelf.tls.peer.secretRef) + && has(self.tls.peer.certManager))' - message: spec.storage.storageClassName cannot be added to or removed from an existing cluster; delete and recreate rule: has(self.storage.storageClassName) == has(oldSelf.storage.storageClassName) diff --git a/controllers/etcdcluster_controller.go b/controllers/etcdcluster_controller.go index 32ab7632..467aa0e1 100644 --- a/controllers/etcdcluster_controller.go +++ b/controllers/etcdcluster_controller.go @@ -137,9 +137,18 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } + // tlsConflict is set when a Certificate this cluster must own is held by + // another controller. That is not fatal to the reconcile — the cluster + // keeps serving on the material it already has — so it is carried down + // to reconcileTLSHandover, which reports it, rather than short-circuiting + // here and leaving the rest of the status to go stale. + var tlsConflict *tlsMaterialConflictError if err := r.reconcileTLSCertificates(ctx, cluster); err != nil { - log.Error(err, "failed to reconcile cert-manager Certificates") - return ctrl.Result{}, err + var isConflict bool + if tlsConflict, isConflict = asTLSMaterialConflict(err); !isConflict { + log.Error(err, "failed to reconcile cert-manager Certificates") + return ctrl.Result{}, err + } } memberList := &lll.EtcdMemberList{} @@ -251,6 +260,23 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) return r.tryDiscoverCluster(ctx, cluster, running) } + // ── TLS handover ─────────────────────────────────────────────────── + // Ahead of any scale decision: if spec.tls has been handed over from + // user-provided Secrets to operator-managed cert-manager issuance, move + // the existing members onto the new material before doing anything + // else. Adding a member on the old material mid-handover would only + // create one more Pod to roll, and doing it while the CAs differ would + // mean the new member cannot authenticate to anyone. + // + // `active` rather than `running`: a dormant member has no Pod to roll, + // but its spec still has to be repointed so it comes back on the right + // material whenever it is resumed. + if res, err := r.reconcileTLSHandover(ctx, cluster, active, tlsConflict); err != nil { + return ctrl.Result{}, err + } else if res != nil { + return *res, nil + } + // ── Scale ────────────────────────────────────────────────────────── // Before deciding to scale either direction, wait for any in-flight // EtcdMember deletion to finish. The EtcdMember finalizer calls @@ -1687,16 +1713,20 @@ type certificateSpec struct { // stable since cert-manager v1.0. // // Create-once rather than Create-or-Patch: the operator never has a -// legitimate reason to mutate an existing Certificate. spec.tls is -// CEL-immutable post-create; the SAN list is derived from immutable -// identifiers (cluster name, namespace, cluster-domain); the issuer -// is locked the same way. Reconciling drift on every loop would fight -// cert-manager's webhook over its defaulted optional fields +// legitimate reason to mutate an existing Certificate. The SAN list is +// derived from immutable identifiers (cluster name, namespace, cluster- +// domain) and the issuer is locked by CEL, so the shape we would patch +// towards is the shape we created. Reconciling drift on every loop would +// fight cert-manager's webhook over its defaulted optional fields // (revisionHistoryLimit, privateKey.rotationPolicy, …) — MergeFrom- // based patches null them out, cert-manager re-defaults them, the // audit log fills up. The simpler invariant is: we own the shape at // creation, after that the resource is cert-manager's territory. // +// The one spec.tls change CEL does allow — the BYO→cert-manager handover +// — only ever adds Certificates the operator did not previously emit, so +// create-once still holds across it. +// // A future operator version that needs to evolve emitted Certificate // shape (e.g. new SAN policy) should ship a one-off migration step // distinct from steady-state reconcile. @@ -1710,9 +1740,25 @@ func (r *EtcdClusterReconciler) ensureCertificate(ctx context.Context, cluster * err := r.Get(ctx, types.NamespacedName{Namespace: cluster.Namespace, Name: spec.name}, existing) switch { case err == nil: - // Already created in a previous reconcile (or by the same - // reconcile that's now retrying). Leave it alone. - return nil + if metav1.IsControlledBy(existing, cluster) { + // Ours, from a previous reconcile (or from the same reconcile + // now retrying). Leave it alone. + return nil + } + // Someone else's object sitting on the name we want. This is the + // realistic shape of the BYO→cert-manager handover: a chart that + // still emits its own Certificates, under names that collide with + // ours whenever the cluster is called `etcd` (chart `etcd-peer` + // vs our `-peer`). + // + // Adopting it would mean mutating an object another controller + // reconciles — Helm/Flux drift correction and this operator would + // fight over it forever — and would silently repoint a live + // cluster at TLS material this operator never issued. Refuse, and + // name the object: dropping it from the chart unblocks the + // handover, and cert-manager leaves the Secret behind so the + // Certificate we then create reissues into it with no gap. + return &tlsMaterialConflictError{kind: "Certificate", name: spec.name} case !errors.IsNotFound(err): return err } diff --git a/controllers/etcdcluster_controller_test.go b/controllers/etcdcluster_controller_test.go index dcf8bc38..55a6f222 100644 --- a/controllers/etcdcluster_controller_test.go +++ b/controllers/etcdcluster_controller_test.go @@ -3805,6 +3805,12 @@ func TestReconcileTLSCertificates_ClusterIssuerKind(t *testing.T) { // Patch on every reconcile that would null those out. Pre-populate a // Certificate with extra spec keys and assert ensureCertificate // leaves it untouched. +// +// The pre-existing Certificate carries this cluster's controller ref, +// because that is what the operator stamps on the ones it creates — and +// ownership is what separates "ours, already created, leave it alone" from +// the conflict case covered by +// TestEnsureCertificate_RefusesForeignOwnedCertificate. func TestEnsureCertificate_DoesNotPatchExistingCertificate(t *testing.T) { ctx := context.Background() cluster := &lll.EtcdCluster{ @@ -3817,6 +3823,13 @@ func TestEnsureCertificate_DoesNotPatchExistingCertificate(t *testing.T) { }) preExisting.SetName("etcd-server") preExisting.SetNamespace("ns") + preExisting.SetOwnerReferences([]metav1.OwnerReference{{ + APIVersion: lll.GroupVersion.String(), + Kind: "EtcdCluster", + Name: "etcd", + UID: types.UID("cluster-uid"), + Controller: ptrBool(true), + }}) preExisting.Object["spec"] = map[string]any{ "secretName": "etcd-server-tls", "commonName": "etcd-server", diff --git a/controllers/etcdmember_controller.go b/controllers/etcdmember_controller.go index c87c69ab..dd0dc5b4 100644 --- a/controllers/etcdmember_controller.go +++ b/controllers/etcdmember_controller.go @@ -449,6 +449,25 @@ func (r *EtcdMemberReconciler) ensurePod(ctx context.Context, member *lll.EtcdMe } member.Status.PodName = pod.Name member.Status.PodUID = string(pod.UID) + // TLS handover: the cluster controller has repointed spec.tls at + // different Secrets, so this Pod is mounting material the member is + // no longer supposed to use. A Pod's volumes are immutable, so the + // only way onto the new material is a rebuild — delete here and let + // the next reconcile recreate, gated as ever on the new Secrets + // actually existing. + // + // This is the one case where an existing Pod is torn down for spec + // drift. Storage, Resources and Version are all deliberately frozen + // per member at creation; TLS is different because holding the + // wrong material does not degrade a member, it isolates it. + if pod.DeletionTimestamp.IsZero() && tlsMountsOutOfDate(pod, member) { + log.FromContext(ctx).Info("member TLS material changed; rebuilding Pod against the new Secrets", + "pod", pod.Name) + if err := r.Delete(ctx, pod); err != nil && !errors.IsNotFound(err) { + return err + } + return nil + } if err := r.reconcileRoleLabel(ctx, pod, member); err != nil { return err } diff --git a/controllers/tls_handover.go b/controllers/tls_handover.go new file mode 100644 index 00000000..7b507c9a --- /dev/null +++ b/controllers/tls_handover.go @@ -0,0 +1,352 @@ +/* +Copyright 2023 Timofey Larkin. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + goerrors "errors" + "fmt" + "sort" + "strings" + "time" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + lll "github.com/cozystack/etcd-operator/api/v1alpha2" +) + +// caCertKey is the Secret key cert-manager writes the issuing CA under, +// and the one etcd is pointed at via --trusted-ca-file / +// --peer-trusted-ca-file. +const caCertKey = "ca.crt" + +// tlsMaterialConflictError reports that a piece of TLS material the +// operator needs to own already exists under another controller's +// ownership. Carried out of ensureCertificate so Reconcile can report it +// as a condition rather than as an opaque reconcile failure. +type tlsMaterialConflictError struct { + kind string + name string +} + +func (e *tlsMaterialConflictError) Error() string { + return fmt.Sprintf("%s %q exists but is not controlled by this EtcdCluster", e.kind, e.name) +} + +// asTLSMaterialConflict unwraps err to a *tlsMaterialConflictError. +func asTLSMaterialConflict(err error) (*tlsMaterialConflictError, bool) { + var conflict *tlsMaterialConflictError + if goerrors.As(err, &conflict) { + return conflict, true + } + return nil, false +} + +// membersNeedingTLSHandover returns the members whose recorded TLS view no +// longer matches what the cluster's spec resolves to. +// +// Every other mirrored field (Storage, Resources, Version) is deliberately +// frozen per member at creation — the cluster controller does not +// re-template existing members when the cluster spec moves. TLS is the one +// exception, because the material a member holds is not a preference but a +// precondition for talking to its peers: leave half the cluster on the old +// CA and there is no cluster, only two halves that cannot authenticate each +// other. +func membersNeedingTLSHandover(cluster *lll.EtcdCluster, members []lll.EtcdMember) []*lll.EtcdMember { + want := deriveMemberTLS(cluster) + var out []*lll.EtcdMember + for i := range members { + m := &members[i] + if !equality.Semantic.DeepEqual(m.Spec.TLS, want) { + out = append(out, m) + } + } + return out +} + +// unreadyTLSSecrets returns a human-readable list of the Secrets named by +// the cluster's current TLS spec that are not yet usable — absent, or +// present but missing a key etcd will be started against. +// +// Existence alone is not a sufficient gate. cert-manager creates the Secret +// before it finishes issuing into it, and a Pod mounting a Secret whose +// tls.key has not landed yet starts etcd against an unreadable file and +// crash-loops. Checking the keys turns that into a wait. +func (r *EtcdClusterReconciler) unreadyTLSSecrets( + ctx context.Context, + cluster *lll.EtcdCluster, + want *lll.EtcdMemberTLS, +) ([]string, error) { + var unready []string + + check := func(name string, keys ...string) error { + sec := &corev1.Secret{} + if err := r.Get(ctx, types.NamespacedName{Namespace: cluster.Namespace, Name: name}, sec); err != nil { + if errors.IsNotFound(err) { + unready = append(unready, name+" (not created yet)") + return nil + } + return err + } + var missing []string + for _, k := range keys { + if len(sec.Data[k]) == 0 { + missing = append(missing, k) + } + } + if len(missing) > 0 { + unready = append(unready, fmt.Sprintf("%s (missing %s)", name, strings.Join(missing, ", "))) + } + return nil + } + + if want != nil { + if want.ClientServerSecretRef != nil { + keys := []string{corev1.TLSCertKey, corev1.TLSPrivateKeyKey} + // ca.crt only matters when the server verifies client certs; + // in server-TLS-only mode etcd is never handed a + // --trusted-ca-file and cert-manager may legitimately omit it. + if want.ClientMTLS { + keys = append(keys, caCertKey) + } + if err := check(want.ClientServerSecretRef.Name, keys...); err != nil { + return nil, err + } + } + if want.PeerSecretRef != nil { + // Peer is always mTLS, so the CA is always load-bearing. + if err := check(want.PeerSecretRef.Name, corev1.TLSCertKey, corev1.TLSPrivateKeyKey, caCertKey); err != nil { + return nil, err + } + } + } + + // The operator's own client identity is not part of the member view, + // but it has to be usable the moment the members come back on the new + // CA — otherwise the roll completes into a cluster the operator can no + // longer dial, and it cannot even observe what it just did. + if name := operatorClientSecretName(cluster); name != "" { + if err := check(name, corev1.TLSCertKey, corev1.TLSPrivateKeyKey); err != nil { + return nil, err + } + } + + sort.Strings(unready) + return unready, nil +} + +// reconcileTLSHandover moves an already-running cluster from user-provided +// TLS Secrets onto operator-managed cert-manager material. +// +// Returns a non-nil *ctrl.Result when it has taken over the reconcile — +// either because it is waiting for material or because it has just rolled +// the members. A nil result means there was nothing to do and the caller +// should carry on. +// +// Sequencing, and why it is a single simultaneous roll rather than a +// rolling restart: the new material is signed by a different CA than the +// old, so an old member and a new member cannot authenticate each other at +// all. Rolling one member at a time would therefore spend the entire roll +// with a split cluster — every intermediate step is a quorum failure, and +// with 3 members a one-at-a-time roll is strictly *worse* than stopping +// everything, because it drags the outage out across three pod startups +// instead of one. So: repoint every member in one pass, let their Pods come +// back together on consistent material, and keep the window as short as the +// slowest pod start. +// +// The conflict argument is the (optional) outcome of certificate emission. +// It is threaded in rather than re-derived so the blocked state is reported +// on the same condition as the rest of the handover. +func (r *EtcdClusterReconciler) reconcileTLSHandover( + ctx context.Context, + cluster *lll.EtcdCluster, + members []lll.EtcdMember, + conflict *tlsMaterialConflictError, +) (*ctrl.Result, error) { + log := log.FromContext(ctx) + + stale := membersNeedingTLSHandover(cluster, members) + + // Conflict first, before the members are even considered. A Certificate + // this cluster must own but does not is worth surfacing whether or not + // any member happens to be drifting right now — reporting Complete while + // the operator cannot own its own material would be a lie of omission. + if conflict != nil { + // Blocked, and only a human can unblock it. Deliberately not fatal + // to the reconcile: the cluster is still serving on the material it + // already holds, and freezing the rest of the loop here would leave + // its readyMembers and health conditions stale — a cluster that + // silently stops reporting is worse than one that reports it cannot + // converge. + log.Info("TLS handover is blocked by a conflicting object", + "kind", conflict.kind, "name", conflict.name) + if setClusterCondition(cluster, lll.ClusterTLSHandover, metav1.ConditionFalse, lll.TLSHandoverBlocked, + fmt.Sprintf("cannot take over %s %q: it exists but another controller owns it — most likely the "+ + "chart that installed this cluster. Stop that chart from emitting it and the operator will "+ + "issue its own; cert-manager leaves the existing Secret in place, so the replacement reissues "+ + "into it without a gap.", conflict.kind, conflict.name)) { + if err := r.statusUpdateTolerateConflict(ctx, cluster); err != nil { + return nil, err + } + } + return nil, nil + } + + if len(stale) == 0 { + // Nothing pending. Only claim completion if we ever said otherwise; + // clusters that were born on cert-manager material never had a + // handover and should not carry a condition about one. + if prev := findClusterCondition(cluster, lll.ClusterTLSHandover); prev != nil { + if setClusterCondition(cluster, lll.ClusterTLSHandover, metav1.ConditionFalse, + lll.TLSHandoverComplete, "all members run on the TLS material named by spec.tls") { + if err := r.statusUpdateTolerateConflict(ctx, cluster); err != nil { + return nil, err + } + } + } + return nil, nil + } + + want := deriveMemberTLS(cluster) + + unready, err := r.unreadyTLSSecrets(ctx, cluster, want) + if err != nil { + return nil, err + } + if len(unready) > 0 { + // Not one member is touched until every piece of the new material + // is on disk. This is the difference between a brief outage and an + // unrecoverable one: repoint the members first and they all come + // back mounting a Secret that does not exist, with the old material + // already out of their spec. + log.Info("TLS handover waiting on material", "unready", unready) + if setClusterCondition(cluster, lll.ClusterTLSHandover, metav1.ConditionTrue, + lll.TLSHandoverAwaitingMaterial, + "waiting for TLS material to be issued: "+strings.Join(unready, "; ")) { + if err := r.statusUpdateTolerateConflict(ctx, cluster); err != nil { + return nil, err + } + } + return &ctrl.Result{RequeueAfter: 10 * time.Second}, nil + } + + names := make([]string, 0, len(stale)) + for _, m := range stale { + orig := m.DeepCopy() + m.Spec.TLS = want.DeepCopy() + if err := r.Patch(ctx, m, client.MergeFrom(orig)); err != nil { + // Partial application is safe to retry: the members already + // patched simply don't come up in the next pass's stale set, + // and the ones that didn't get patched are picked up again. + return nil, err + } + names = append(names, m.Name) + } + sort.Strings(names) + + log.Info("TLS handover: repointed members at operator-managed material; their Pods will be rebuilt", + "members", names) + if setClusterCondition(cluster, lll.ClusterTLSHandover, metav1.ConditionTrue, lll.TLSHandoverRollingMembers, + fmt.Sprintf("rolling %d member(s) onto operator-managed TLS material: %s", + len(names), strings.Join(names, ", "))) { + if err := r.statusUpdateTolerateConflict(ctx, cluster); err != nil { + return nil, err + } + } + return &ctrl.Result{RequeueAfter: 5 * time.Second}, nil +} + +// statusUpdateTolerateConflict writes the cluster status, treating a +// conflict as success — the next reconcile re-derives the same condition +// from a fresh read. Keeps the handover from turning an optimistic- +// concurrency retry into a reconcile error. +func (r *EtcdClusterReconciler) statusUpdateTolerateConflict(ctx context.Context, cluster *lll.EtcdCluster) error { + if err := r.Status().Update(ctx, cluster); err != nil && !errors.IsConflict(err) { + return err + } + return nil +} + +// findClusterCondition returns the named condition, or nil. +func findClusterCondition(cluster *lll.EtcdCluster, condType string) *metav1.Condition { + for i := range cluster.Status.Conditions { + if cluster.Status.Conditions[i].Type == condType { + return &cluster.Status.Conditions[i] + } + } + return nil +} + +// podTLSSecretNames reports the Secret names a Pod actually mounts for the +// client and peer planes. Empty string means the plane is not mounted. +func podTLSSecretNames(pod *corev1.Pod) (clientSecret, peerSecret string) { + for _, v := range pod.Spec.Volumes { + if v.Secret == nil { + continue + } + switch v.Name { + case "tls-client": + clientSecret = v.Secret.SecretName + case "tls-peer": + peerSecret = v.Secret.SecretName + } + } + return clientSecret, peerSecret +} + +// tlsMountsOutOfDate reports whether a running Pod mounts different TLS +// Secrets than its member spec now names — the observable trace of a +// handover that has repointed the spec but not yet rebuilt the Pod. +// +// Scoped strictly to Secret *names*. Content changes (a cert-manager +// renewal writing a fresh leaf into the same Secret) are not a reason to +// rebuild anything: the kubelet refreshes the projected volume in place and +// etcd picks the new leaf up on subsequent handshakes. +// +// Only Pods this operator built are candidates. A Pod carrying neither of +// the operator's TLS volumes is either plaintext (nothing to compare) or a +// shape the operator did not author — an adopted member from etcd-migrate +// keeps the legacy StatefulSet's volume names, and adoption deliberately +// never restarts it. Treating that as drift would delete every adopted Pod +// on the first reconcile after a migration, which is precisely the outcome +// in-place adoption exists to avoid. Such members are left alone; a +// handover rolls them only when they are rolled onto the operator's own +// Pod shape. +func tlsMountsOutOfDate(pod *corev1.Pod, member *lll.EtcdMember) bool { + gotClient, gotPeer := podTLSSecretNames(pod) + if gotClient == "" && gotPeer == "" { + return false + } + + var wantClient, wantPeer string + if member.Spec.TLS != nil { + if member.Spec.TLS.ClientServerSecretRef != nil { + wantClient = member.Spec.TLS.ClientServerSecretRef.Name + } + if member.Spec.TLS.PeerSecretRef != nil { + wantPeer = member.Spec.TLS.PeerSecretRef.Name + } + } + return gotClient != wantClient || gotPeer != wantPeer +} diff --git a/controllers/tls_handover_test.go b/controllers/tls_handover_test.go new file mode 100644 index 00000000..d982932b --- /dev/null +++ b/controllers/tls_handover_test.go @@ -0,0 +1,488 @@ +/* +Copyright 2023 Timofey Larkin. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package controllers + +import ( + "context" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + lll "github.com/cozystack/etcd-operator/api/v1alpha2" +) + +// A cluster mid-handover: spec.tls names cert-manager issuance, while the +// members still carry the BYO Secret refs they were created with. +func handoverCluster() *lll.EtcdCluster { + return &lll.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "etcd", Namespace: "ns", UID: "cluster-uid"}, + Spec: lll.EtcdClusterSpec{ + Replicas: ptrInt32(3), + Version: "3.5.17", + TLS: &lll.EtcdClusterTLS{ + Client: &lll.ClientTLS{ + CertManager: &lll.ClientCertManagerTLS{ + ServerIssuerRef: lll.IssuerReference{Name: "etcd-issuer"}, + }, + }, + Peer: &lll.PeerTLS{ + CertManager: &lll.PeerCertManagerTLS{ + IssuerRef: lll.IssuerReference{Name: "etcd-peer-issuer"}, + }, + }, + }, + }, + } +} + +// byoMember is a member still pinned to the chart-provided Secrets. +func byoMember(name string) *lll.EtcdMember { + return &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "ns"}, + Spec: lll.EtcdMemberSpec{ + ClusterName: "etcd", + Version: "3.5.17", + TLS: &lll.EtcdMemberTLS{ + ClientServerSecretRef: &corev1.LocalObjectReference{Name: "legacy-server-tls"}, + PeerSecretRef: &corev1.LocalObjectReference{Name: "legacy-peer-tls"}, + }, + }, + } +} + +func tlsSecret(name string, keys ...string) *corev1.Secret { + data := map[string][]byte{} + for _, k := range keys { + data[k] = []byte("x") + } + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "ns"}, + Data: data, + } +} + +func handoverCondition(t *testing.T, c client.Client) *metav1.Condition { + t.Helper() + got := mustGet(t, c, "etcd", "ns", &lll.EtcdCluster{}) + return findClusterCondition(got, lll.ClusterTLSHandover) +} + +// Until every Secret named by the new spec is populated, not a single +// member may be repointed. Repointing first would strand the whole cluster +// on a Secret that does not exist, with the old material already gone from +// its spec. +func TestTLSHandover_WaitsForMaterialBeforeTouchingMembers(t *testing.T) { + ctx := context.Background() + cluster := handoverCluster() + m1, m2 := byoMember("etcd-aaa"), byoMember("etcd-bbb") + c, s := newTestClient(t, cluster, m1, m2) + r := &EtcdClusterReconciler{Client: c, Scheme: s} + + res, err := r.reconcileTLSHandover(ctx, cluster, []lll.EtcdMember{*m1, *m2}, nil) + if err != nil { + t.Fatalf("reconcileTLSHandover: %v", err) + } + if res == nil { + t.Fatalf("expected the handover to take over the reconcile while material is missing") + } + + for _, name := range []string{"etcd-aaa", "etcd-bbb"} { + got := mustGet(t, c, name, "ns", &lll.EtcdMember{}) + if got.Spec.TLS.PeerSecretRef.Name != "legacy-peer-tls" { + t.Fatalf("member %s was repointed before its material existed: %+v", name, got.Spec.TLS) + } + } + + cond := handoverCondition(t, c) + if cond == nil || cond.Status != metav1.ConditionTrue || cond.Reason != lll.TLSHandoverAwaitingMaterial { + t.Fatalf("want TLSHandover=True/AwaitingMaterial, got %+v", cond) + } + if !strings.Contains(cond.Message, "etcd-peer-tls") { + t.Fatalf("condition should name the material it is waiting on, got %q", cond.Message) + } +} + +// A Secret that exists but has not been issued into yet is not ready. etcd +// started against a half-written Secret crash-loops. +func TestTLSHandover_SecretPresentButUnpopulatedIsNotReady(t *testing.T) { + ctx := context.Background() + cluster := handoverCluster() + m := byoMember("etcd-aaa") + c, s := newTestClient(t, cluster, m, + tlsSecret("etcd-server-tls", corev1.TLSCertKey, corev1.TLSPrivateKeyKey), + // peer Secret exists but cert-manager has not written the key yet + tlsSecret("etcd-peer-tls", corev1.TLSCertKey, caCertKey), + ) + r := &EtcdClusterReconciler{Client: c, Scheme: s} + + res, err := r.reconcileTLSHandover(ctx, cluster, []lll.EtcdMember{*m}, nil) + if err != nil { + t.Fatalf("reconcileTLSHandover: %v", err) + } + if res == nil { + t.Fatalf("expected a requeue while the peer Secret is incomplete") + } + got := mustGet(t, c, "etcd-aaa", "ns", &lll.EtcdMember{}) + if got.Spec.TLS.PeerSecretRef.Name != "legacy-peer-tls" { + t.Fatalf("member repointed against an unpopulated Secret: %+v", got.Spec.TLS) + } + cond := handoverCondition(t, c) + if cond == nil || cond.Reason != lll.TLSHandoverAwaitingMaterial { + t.Fatalf("want AwaitingMaterial, got %+v", cond) + } + if !strings.Contains(cond.Message, corev1.TLSPrivateKeyKey) { + t.Fatalf("condition should name the missing key, got %q", cond.Message) + } +} + +// Once the material is ready, every member is repointed in the same pass. +// A staggered roll would leave old and new members unable to authenticate +// to each other for the whole duration, which is a longer outage, not a +// shorter one. +func TestTLSHandover_RepointsEveryMemberInOnePass(t *testing.T) { + ctx := context.Background() + cluster := handoverCluster() + m1, m2, m3 := byoMember("etcd-aaa"), byoMember("etcd-bbb"), byoMember("etcd-ccc") + c, s := newTestClient(t, cluster, m1, m2, m3, + tlsSecret("etcd-server-tls", corev1.TLSCertKey, corev1.TLSPrivateKeyKey), + tlsSecret("etcd-peer-tls", corev1.TLSCertKey, corev1.TLSPrivateKeyKey, caCertKey), + ) + r := &EtcdClusterReconciler{Client: c, Scheme: s} + + res, err := r.reconcileTLSHandover(ctx, cluster, []lll.EtcdMember{*m1, *m2, *m3}, nil) + if err != nil { + t.Fatalf("reconcileTLSHandover: %v", err) + } + if res == nil { + t.Fatalf("expected the handover to take over the reconcile after repointing") + } + + for _, name := range []string{"etcd-aaa", "etcd-bbb", "etcd-ccc"} { + got := mustGet(t, c, name, "ns", &lll.EtcdMember{}) + if got.Spec.TLS.ClientServerSecretRef.Name != "etcd-server-tls" { + t.Errorf("member %s client ref = %q, want etcd-server-tls", name, got.Spec.TLS.ClientServerSecretRef.Name) + } + if got.Spec.TLS.PeerSecretRef.Name != "etcd-peer-tls" { + t.Errorf("member %s peer ref = %q, want etcd-peer-tls", name, got.Spec.TLS.PeerSecretRef.Name) + } + } + + cond := handoverCondition(t, c) + if cond == nil || cond.Status != metav1.ConditionTrue || cond.Reason != lll.TLSHandoverRollingMembers { + t.Fatalf("want TLSHandover=True/RollingMembers, got %+v", cond) + } +} + +// Each member must get its own copy of the derived TLS view; sharing one +// pointer across members would make a later mutation of one silently +// rewrite the others. +func TestTLSHandover_MembersDoNotShareTLSPointer(t *testing.T) { + ctx := context.Background() + cluster := handoverCluster() + m1, m2 := byoMember("etcd-aaa"), byoMember("etcd-bbb") + c, s := newTestClient(t, cluster, m1, m2, + tlsSecret("etcd-server-tls", corev1.TLSCertKey, corev1.TLSPrivateKeyKey), + tlsSecret("etcd-peer-tls", corev1.TLSCertKey, corev1.TLSPrivateKeyKey, caCertKey), + ) + r := &EtcdClusterReconciler{Client: c, Scheme: s} + + members := []lll.EtcdMember{*m1, *m2} + if _, err := r.reconcileTLSHandover(ctx, cluster, members, nil); err != nil { + t.Fatalf("reconcileTLSHandover: %v", err) + } + if members[0].Spec.TLS == members[1].Spec.TLS { + t.Fatalf("members share the same *EtcdMemberTLS; each needs its own copy") + } +} + +// A conflict is reported, not acted on, and above all does not stop the +// reconcile: the cluster is still serving on its existing material and its +// health status has to keep flowing. +func TestTLSHandover_ConflictReportedWithoutTouchingMembers(t *testing.T) { + ctx := context.Background() + cluster := handoverCluster() + m := byoMember("etcd-aaa") + c, s := newTestClient(t, cluster, m) + r := &EtcdClusterReconciler{Client: c, Scheme: s} + + conflict := &tlsMaterialConflictError{kind: "Certificate", name: "etcd-peer"} + res, err := r.reconcileTLSHandover(ctx, cluster, []lll.EtcdMember{*m}, conflict) + if err != nil { + t.Fatalf("a conflict must not fail the reconcile: %v", err) + } + if res != nil { + t.Fatalf("a conflict must not take over the reconcile; the rest of the loop still has work to do") + } + + got := mustGet(t, c, "etcd-aaa", "ns", &lll.EtcdMember{}) + if got.Spec.TLS.PeerSecretRef.Name != "legacy-peer-tls" { + t.Fatalf("member repointed despite the conflict: %+v", got.Spec.TLS) + } + cond := handoverCondition(t, c) + if cond == nil || cond.Status != metav1.ConditionFalse || cond.Reason != lll.TLSHandoverBlocked { + t.Fatalf("want TLSHandover=False/Blocked, got %+v", cond) + } + if !strings.Contains(cond.Message, "etcd-peer") { + t.Fatalf("condition should name the conflicting object, got %q", cond.Message) + } +} + +// A cluster that was born on cert-manager material never had a handover +// and must not acquire a condition claiming one completed. +func TestTLSHandover_NoConditionWhenNothingEverDrifted(t *testing.T) { + ctx := context.Background() + cluster := handoverCluster() + aligned := byoMember("etcd-aaa") + aligned.Spec.TLS = deriveMemberTLS(cluster) + c, s := newTestClient(t, cluster, aligned) + r := &EtcdClusterReconciler{Client: c, Scheme: s} + + res, err := r.reconcileTLSHandover(ctx, cluster, []lll.EtcdMember{*aligned}, nil) + if err != nil { + t.Fatalf("reconcileTLSHandover: %v", err) + } + if res != nil { + t.Fatalf("aligned cluster should not take over the reconcile") + } + if cond := handoverCondition(t, c); cond != nil { + t.Fatalf("unexpected TLSHandover condition on a cluster that never drifted: %+v", cond) + } +} + +// Once the roll lands, the in-flight condition resolves to Complete rather +// than being left permanently True. +func TestTLSHandover_SettlesToComplete(t *testing.T) { + ctx := context.Background() + cluster := handoverCluster() + aligned := byoMember("etcd-aaa") + aligned.Spec.TLS = deriveMemberTLS(cluster) + // Simulate the previous pass having reported the roll. + setClusterCondition(cluster, lll.ClusterTLSHandover, metav1.ConditionTrue, + lll.TLSHandoverRollingMembers, "rolling") + c, s := newTestClient(t, cluster, aligned) + r := &EtcdClusterReconciler{Client: c, Scheme: s} + + if _, err := r.reconcileTLSHandover(ctx, cluster, []lll.EtcdMember{*aligned}, nil); err != nil { + t.Fatalf("reconcileTLSHandover: %v", err) + } + cond := handoverCondition(t, c) + if cond == nil || cond.Status != metav1.ConditionFalse || cond.Reason != lll.TLSHandoverComplete { + t.Fatalf("want TLSHandover=False/Complete, got %+v", cond) + } +} + +// ensureCertificate must never adopt a Certificate another controller owns +// — that is the chart-collision case, and taking it over would mean two +// controllers reconciling one object forever. +func TestEnsureCertificate_RefusesForeignOwnedCertificate(t *testing.T) { + ctx := context.Background() + cluster := handoverCluster() + + foreign := &unstructured.Unstructured{} + foreign.SetGroupVersionKind(schema.GroupVersionKind{Group: "cert-manager.io", Version: "v1", Kind: "Certificate"}) + foreign.SetName("etcd-peer") + foreign.SetNamespace("ns") + foreign.SetOwnerReferences([]metav1.OwnerReference{{ + APIVersion: "helm.toolkit.fluxcd.io/v2", + Kind: "HelmRelease", + Name: "etcd", + UID: "helm-uid", + Controller: ptrBool(true), + }}) + + c, s := newTestClient(t, cluster) + if err := c.Create(ctx, foreign); err != nil { + t.Fatalf("seed foreign Certificate: %v", err) + } + r := &EtcdClusterReconciler{Client: c, Scheme: s} + + err := r.ensureCertificate(ctx, cluster, certificateSpec{ + name: "etcd-peer", + secretName: "etcd-peer-tls", + commonName: "etcd-peer", + issuerRef: lll.IssuerReference{Name: "etcd-peer-issuer"}, + }) + conflict, ok := asTLSMaterialConflict(err) + if !ok { + t.Fatalf("want a tlsMaterialConflictError, got %v", err) + } + if conflict.name != "etcd-peer" || conflict.kind != "Certificate" { + t.Fatalf("conflict does not identify the object: %+v", conflict) + } +} + +func TestTLSMountsOutOfDate(t *testing.T) { + podWith := func(clientSecret, peerSecret string) *corev1.Pod { + p := &corev1.Pod{} + if clientSecret != "" { + p.Spec.Volumes = append(p.Spec.Volumes, corev1.Volume{ + Name: "tls-client", + VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: clientSecret}}, + }) + } + if peerSecret != "" { + p.Spec.Volumes = append(p.Spec.Volumes, corev1.Volume{ + Name: "tls-peer", + VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: peerSecret}}, + }) + } + return p + } + memberWith := func(tls *lll.EtcdMemberTLS) *lll.EtcdMember { + return &lll.EtcdMember{Spec: lll.EtcdMemberSpec{TLS: tls}} + } + refs := func(clientSecret, peerSecret string) *lll.EtcdMemberTLS { + out := &lll.EtcdMemberTLS{} + if clientSecret != "" { + out.ClientServerSecretRef = &corev1.LocalObjectReference{Name: clientSecret} + } + if peerSecret != "" { + out.PeerSecretRef = &corev1.LocalObjectReference{Name: peerSecret} + } + return out + } + + cases := []struct { + name string + pod *corev1.Pod + mem *lll.EtcdMember + want bool + }{ + {"plaintext cluster is never out of date", podWith("", ""), memberWith(nil), false}, + {"matching refs", podWith("s", "p"), memberWith(refs("s", "p")), false}, + {"peer secret renamed", podWith("s", "old-p"), memberWith(refs("s", "p")), true}, + {"client secret renamed", podWith("old-s", "p"), memberWith(refs("s", "p")), true}, + {"both renamed", podWith("old-s", "old-p"), memberWith(refs("s", "p")), true}, + // --peer-auto-tls mounts nothing for the peer plane; that is not drift. + {"peer-auto-tls", podWith("s", ""), memberWith(&lll.EtcdMemberTLS{ + ClientServerSecretRef: &corev1.LocalObjectReference{Name: "s"}, + PeerAutoTLS: true, + }), false}, + // An adopted member keeps the legacy StatefulSet's volume names and + // is never restarted by adoption. Its Pod carries none of the + // operator's TLS volumes even though the member spec names Secrets; + // calling that drift would delete every adopted Pod on the first + // reconcile after a migration. + {"adopted pod with foreign volume names", adoptedPod(), memberWith(refs("s", "p")), false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := tlsMountsOutOfDate(tc.pod, tc.mem); got != tc.want { + t.Fatalf("tlsMountsOutOfDate = %v, want %v", got, tc.want) + } + }) + } +} + +// tlsPod builds a member-owned Pod mounting the named TLS Secrets. +func tlsPod(name, clientSecret, peerSecret string, memberUID types.UID) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, Namespace: "ns", + OwnerReferences: []metav1.OwnerReference{{Kind: "EtcdMember", Name: name, UID: memberUID}}, + }, + Spec: corev1.PodSpec{Volumes: []corev1.Volume{ + {Name: "tls-client", VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{SecretName: clientSecret}}}, + {Name: "tls-peer", VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{SecretName: peerSecret}}}, + }}, + } +} + +// operatorManaged is the member-side TLS view after a handover. +func operatorManaged() *lll.EtcdMemberTLS { + return &lll.EtcdMemberTLS{ + ClientServerSecretRef: &corev1.LocalObjectReference{Name: "etcd-server-tls"}, + PeerSecretRef: &corev1.LocalObjectReference{Name: "etcd-peer-tls"}, + } +} + +// A Pod mounting superseded Secrets is torn down so it can be rebuilt. +// Pod volumes are immutable, so a rebuild is the only route onto new +// material. +func TestEnsurePod_RebuildsPodWhenTLSSecretsChange(t *testing.T) { + ctx := context.Background() + member := byoMember("etcd-aaa") + member.UID = "member-uid" + member.Spec.TLS = operatorManaged() + + pod := tlsPod("etcd-aaa", "legacy-server-tls", "legacy-peer-tls", "member-uid") + c, s := newTestClient(t, member, pod) + r := &EtcdMemberReconciler{Client: c, Scheme: s} + + if err := r.ensurePod(ctx, member); err != nil { + t.Fatalf("ensurePod: %v", err) + } + err := c.Get(ctx, types.NamespacedName{Name: "etcd-aaa", Namespace: "ns"}, &corev1.Pod{}) + if !apierrors.IsNotFound(err) { + t.Fatalf("Pod mounting superseded TLS Secrets was not deleted (err=%v)", err) + } +} + +// Once rebuilt on matching Secrets the Pod must be left alone, or the +// member spins in a delete loop and never becomes Ready. +func TestEnsurePod_LeavesPodAloneWhenTLSMatches(t *testing.T) { + ctx := context.Background() + member := byoMember("etcd-aaa") + member.UID = "member-uid" + member.Spec.TLS = operatorManaged() + + pod := tlsPod("etcd-aaa", "etcd-server-tls", "etcd-peer-tls", "member-uid") + c, s := newTestClient(t, member, pod) + r := &EtcdMemberReconciler{Client: c, Scheme: s} + + if err := r.ensurePod(ctx, member); err != nil { + t.Fatalf("ensurePod: %v", err) + } + if err := c.Get(ctx, types.NamespacedName{Name: "etcd-aaa", Namespace: "ns"}, &corev1.Pod{}); err != nil { + t.Fatalf("Pod on current TLS material was deleted: %v", err) + } +} + +// A conflict is reported even when no member happens to be drifting. +// Reporting Complete while the operator cannot own its own material would +// be a lie of omission. +func TestTLSHandover_ConflictReportedEvenWhenMembersAreAligned(t *testing.T) { + ctx := context.Background() + cluster := handoverCluster() + aligned := byoMember("etcd-aaa") + aligned.Spec.TLS = deriveMemberTLS(cluster) + c, s := newTestClient(t, cluster, aligned) + r := &EtcdClusterReconciler{Client: c, Scheme: s} + + conflict := &tlsMaterialConflictError{kind: "Certificate", name: "etcd-server"} + if _, err := r.reconcileTLSHandover(ctx, cluster, []lll.EtcdMember{*aligned}, conflict); err != nil { + t.Fatalf("reconcileTLSHandover: %v", err) + } + cond := handoverCondition(t, c) + if cond == nil || cond.Reason != lll.TLSHandoverBlocked { + t.Fatalf("want Blocked even with aligned members, got %+v", cond) + } +} + +// adoptedPod mimics a member adopted in place by etcd-migrate: the legacy +// StatefulSet's volume names, which the operator never authored. +func adoptedPod() *corev1.Pod { + return &corev1.Pod{Spec: corev1.PodSpec{Volumes: []corev1.Volume{ + {Name: "etcd-client-certs", VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{SecretName: "legacy-server-tls"}}}, + {Name: "etcd-peer-certs", VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{SecretName: "legacy-peer-tls"}}}, + }}} +} diff --git a/docs/concepts.md b/docs/concepts.md index 6cd535ba..54f449c4 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -177,7 +177,7 @@ The `PodDisruptionBudget` *is* auto-emitted now — see the [PodDisruptionBudget ### Apiserver-enforced validation -Four CEL `x-kubernetes-validations` rules on `EtcdClusterSpec` are evaluated at admission time. **k8s 1.29+ is the safe floor**: CEL CRD validation (`CustomResourceValidationExpressions`) went GA in 1.29, and the `quantity()` extension function used by two of the rules was added in 1.28. The CEL gate was beta-on-by-default from 1.25, so 1.28 *may* work in practice — but 1.29 is the first version where both pieces are GA and the project doesn't have to chase feature-gate state across releases. +The CEL `x-kubernetes-validations` rules on `EtcdClusterSpec` listed below are evaluated at admission time. **k8s 1.29+ is the safe floor**: CEL CRD validation (`CustomResourceValidationExpressions`) went GA in 1.29, and the `quantity()` extension function used by two of the rules was added in 1.28. The CEL gate was beta-on-by-default from 1.25, so 1.28 *may* work in practice — but 1.29 is the first version where both pieces are GA and the project doesn't have to chase feature-gate state across releases. | Rule | When | Why | |---|---|---| @@ -188,7 +188,8 @@ Four CEL `x-kubernetes-validations` rules on `EtcdClusterSpec` are evaluated at | `storage.storageClassName` cannot be added or removed | UPDATE | `PersistentVolumeClaim.spec.storageClassName` is immutable; honouring a mid-life add/remove would require rolling every PVC. | | `storage.storageClassName` value immutable | UPDATE | Same reason — the StorageClass chosen at cluster creation is the only one PVCs will ever carry. | | `tls` cannot be added or removed | UPDATE | Toggling TLS on an existing cluster is a rolling restart that has to land on the operator's etcd client and every member Pod in lockstep; not implemented. | -| `tls` subtree immutable | UPDATE | Same reason — secret-ref swaps, mTLS-flip via `operatorClientSecretRef`, peer-only ↔ both toggles are all in-place rolling changes that v1 doesn't perform. | +| `tls.client` / `tls.peer` cannot be added or removed | UPDATE | Turning one plane on or off mid-life is the same lockstep rolling problem as adding `tls` wholesale. | +| `tls.client` / `tls.peer` immutable, except BYO → `certManager` | UPDATE | Secret-ref swaps and mTLS flips via `operatorClientSecretRef` are in-place rolling changes v1 doesn't perform. The single exception is handing the material over to operator-managed cert-manager issuance, which the operator *can* drive end to end — see [Handing TLS over to the operator](operations.md#handing-tls-over-to-the-operator). It is one-way, and must preserve the client-mTLS posture. | These rules live in the CRD itself; the apiserver enforces them with no separate webhook, no cert-manager, no extra Deployment. Errors come back as standard apiserver admission rejections (`kubectl apply` prints the rule's `message` field). @@ -255,7 +256,11 @@ Because the operator never stamps these annotations, every rolled or replaced me ## TLS -`spec.tls` configures transport-layer security for the cluster's two etcd surfaces: the client API (port 2379) and the peer API (port 2380). Each subtree is independently optional — you can opt one surface into TLS without the other. The whole `tls` subtree is immutable post-create (see the validation table above): toggling TLS on an existing cluster is a rolling change that v1 doesn't perform, so the policy is delete-and-recreate. +`spec.tls` configures transport-layer security for the cluster's two etcd surfaces: the client API (port 2379) and the peer API (port 2380). Each subtree is independently optional — you can opt one surface into TLS without the other. The `tls` subtree is immutable post-create (see the validation table above): toggling TLS on an existing cluster is a rolling change that v1 doesn't perform, so the policy is delete-and-recreate. + +The one exception is the **source** of the material. A cluster running on user-provided Secrets can be handed over to operator-managed cert-manager issuance in place — `client.serverSecretRef` → `client.certManager`, `peer.secretRef` → `peer.certManager` — because that is a change the operator can drive safely from end to end: it issues the new Certificates, waits for cert-manager to populate every Secret, then repoints and rolls all members together. The runbook is [Handing TLS over to the operator](operations.md#handing-tls-over-to-the-operator). + +The exception is deliberately narrow. It is **one-way** (there is no `certManager` → `secretRef`: the operator would be handing a live cluster to material it cannot verify, while the Certificates it owns get garbage-collected out from under it), it may not change whether client mTLS is in effect, and it does not let a plane be switched on or off. Everything else is still delete-and-recreate. Material can come from one of two sources per subtree, mutually exclusive: diff --git a/docs/installation.md b/docs/installation.md index d8c6ab4c..3b9b52ed 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -241,7 +241,7 @@ Required SANs on the per-cluster peer cert: both `*...svc` AND `*.< Server cert EKU **must include `serverAuth` AND `clientAuth`** (the etcd grpc-gateway loopback presents the server cert as a client cert when self-dialing; the server's `--trusted-ca-file` then verifies it with `ExtKeyUsageClientAuth`). Peer cert EKU must include both because peer is symmetric. Operator-client cert needs only `clientAuth`. -The `spec.tls` subtree is immutable post-create — flipping TLS on or off on an existing cluster is delete-and-recreate. +The `spec.tls` subtree is immutable post-create — flipping TLS on or off on an existing cluster is delete-and-recreate. The one permitted change is handing the material over from these Secrets to operator-managed cert-manager issuance; see [Handing TLS over to the operator](operations.md#handing-tls-over-to-the-operator). ## Image versions diff --git a/docs/operations.md b/docs/operations.md index 48649db8..43ac3aa8 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -205,6 +205,68 @@ kubectl patch etcdcluster.etcd-operator.cozystack.io -n --subresourc This pushes the cluster into the terminal-error state immediately. Recovery follows the relevant condition arm above (delete-and-recreate for pre-bootstrap, spec-edit for post-bootstrap). +## Handing TLS over to the operator + +A cluster running on user-provided TLS Secrets can be moved onto operator-managed cert-manager issuance in place. This is the only permitted change to `spec.tls` after create — see the [validation table](concepts.md#apiserver-enforced-validation). + +**It costs a brief outage.** The new material is signed by a different CA, so an old member and a new one cannot authenticate to each other at all. The operator therefore repoints every member in one pass and lets their Pods come back together, rather than rolling one at a time — a staggered roll would spend its entire duration with a split, quorum-less cluster instead of a single pod-restart window. + +### 1. Free up the names, if something else holds them + +The operator emits up to three Certificates — `Certificate/-server`, `Certificate/-peer`, and `Certificate/-operator-client` when client mTLS is on (see [census of certs](concepts.md#census-of-certs)). If a Helm chart already installs Certificates under any of those names — which it does whenever the cluster is called `etcd` — the operator refuses to touch them and reports: + +```text +TLSHandover=False reason=Blocked + cannot take over Certificate "etcd-peer": it exists but another controller owns it … +``` + +Only the first collision is reported per pass, so expect to clear them one at a time: fix the named object, and the next reconcile either proceeds or names the next one. + +Stop the chart from emitting them (drop the templates, re-reconcile). cert-manager leaves the **Secret** in place when its Certificate is deleted, so there is no gap: the Certificate the operator then creates reissues into the same Secret. + +### 2. Flip the spec + +```yaml +spec: + tls: + client: + certManager: + serverIssuerRef: {name: etcd-issuer} + # Required if — and only if — the cluster previously set + # operatorClientSecretRef. The handover may not change whether + # client mTLS is in effect; CEL rejects a posture flip. + operatorClientIssuerRef: {name: etcd-issuer} + peer: + certManager: + issuerRef: {name: etcd-peer-issuer} +``` + +Drop `serverSecretRef` / `operatorClientSecretRef` / `secretRef` in the same edit — they are mutually exclusive with `certManager`, and CEL rejects an object carrying both. + +### 3. Watch it land + +```sh +kubectl get etcdcluster -n \ + -o jsonpath='{range .status.conditions[?(@.type=="TLSHandover")]}{.status} {.reason} {.message}{"\n"}{end}' +``` + +| Reason | Meaning | +|---|---| +| `AwaitingMaterial` | Certificates requested; waiting for cert-manager to write every key. **No member has been touched yet** — the cluster is still serving on its old material. | +| `RollingMembers` | Every member has been repointed; their Pods are being rebuilt. This is the outage window. | +| `Complete` | Every member runs on operator-managed material. | +| `Blocked` | Step 1 is not done. Nothing has been touched. | + +A handover that stalls in `AwaitingMaterial` is a cert-manager problem, not an operator one — the message names the Secret and the key it is waiting on. Check the `Certificate` and its `CertificateRequest`. + +### Members adopted in place + +Members adopted by `etcd-migrate` keep the Pod the legacy StatefulSet created — adoption rewrites labels and owner refs but deliberately never restarts them. The operator does not roll those Pods during a handover: it only rebuilds Pods it authored itself, so an adopted member keeps running on its old material until you roll it. Once rolled onto the operator's own Pod shape it participates normally. Check with `kubectl get pod -o jsonpath='{.spec.volumes[*].name}'` — an operator-built Pod carries `tls-client` / `tls-peer`. + +### Going back + +There is no reverse. `certManager` → `secretRef` is CEL-rejected: the operator would be handing a live cluster to material it cannot verify, while the Certificates it owns get garbage-collected out from under it. If you need BYO Secrets again, it is delete-and-recreate. + ## Broken member Recovery from a permanently broken **PVC-backed** member (e.g. PVC lost, node retired) is currently manual. Memory-backed members are auto-replaced on Pod loss — see [Memory-backed clusters](#memory-backed-clusters) above. For PVC-backed clusters the `isBroken` predicate stays a stub; auto-replacement is not wired up (see [concepts](concepts.md#what-is-not-in-the-design)).