fix(main): make peer-auto-tls a reserved annotation, not typed spec

Address review feedback (PR #330): an unauthenticated peer plane must not
be a discoverable, CEL-blessed option for new clusters.

- Drop spec.tls.peer.autoTLS (and the PeerAutoTLS type); revert the PeerTLS
  CEL rule to the two-way has(secretRef) != has(certManager) union.
- Carry legacy --peer-auto-tls forward on a cluster-level reserved annotation
  etcd-operator.cozystack.io/peer-auto-tls instead. deriveMemberTLS reads it
  (superseded by an explicit peer secretRef/certManager) and propagates it to
  every member it builds; clusterPeerScheme treats it as https.
- etcd-migrate stamps the annotation and raises it as a SecurityWarning,
  rendered with a loud marker in the plan AND re-surfaced in the post-apply
  summary so an unauthenticated peer plane can't be adopted unnoticed.
- Document the mode, carry-forward and off-ramp in docs/migration.md.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: androndo

api/v1alpha2/cel_validation_test.go

@@ -482,36 +482,6 @@ func TestCEL_TLSPeerCertManagerAndSecretRefMutuallyExclusive(t *testing.T) {
 	}
 }
 
-// TestCEL_TLSPeerAutoTLS: autoTLS alone is accepted; autoTLS combined with
-// another peer source is rejected by the exactly-one-of rule.
-func TestCEL_TLSPeerAutoTLS(t *testing.T) {
-	skipIfNoEnvtest(t)
-	ctx := context.Background()
-
-	// autoTLS alone — accepted.
-	ok := validCluster("tls-peer-autotls")
-	ok.Spec.TLS = &lll.EtcdClusterTLS{Peer: &lll.PeerTLS{AutoTLS: &lll.PeerAutoTLS{}}}
-	if err := k8s.Create(ctx, ok); err != nil {
-		t.Fatalf("apiserver rejected peer.autoTLS alone: %v", err)
-	}
-	_ = k8s.Delete(ctx, ok)
-
-	// autoTLS + secretRef — rejected.
-	bad := validCluster("tls-peer-autotls-both")
-	bad.Spec.TLS = &lll.EtcdClusterTLS{Peer: &lll.PeerTLS{
-		AutoTLS:   &lll.PeerAutoTLS{},
-		SecretRef: &corev1.LocalObjectReference{Name: "fake-peer-tls"},
-	}}
-	err := k8s.Create(ctx, bad)
-	if err == nil {
-		_ = k8s.Delete(ctx, bad)
-		t.Fatalf("apiserver accepted peer.autoTLS + peer.secretRef; expected rejection")
-	}
-	if !strings.Contains(err.Error(), "exactly one of spec.tls.peer.secretRef") {
-		t.Fatalf("error did not mention peer mutual exclusion: %v", err)
-	}
-}
-
 // TestCEL_HappyPathAccepts is a negative-side guard: a fully valid
 // cluster spec must pass the apiserver. Catches accidental rule
 // inversions and over-broad CEL expressions.

api/v1alpha2/etcdcluster_types.go

@@ -194,7 +194,7 @@ type ClientCertManagerTLS struct {
 // PeerTLS configures TLS for the etcd peer API. When PeerTLS is set, peer
 // is always mTLS.
 //
-// +kubebuilder:validation:XValidation:rule="[has(self.secretRef), has(self.certManager), has(self.autoTLS)].filter(x, x).size() == 1",message="exactly one of spec.tls.peer.secretRef, spec.tls.peer.certManager or spec.tls.peer.autoTLS must be set"
+// +kubebuilder:validation:XValidation:rule="has(self.secretRef) != has(self.certManager)",message="exactly one of spec.tls.peer.secretRef or spec.tls.peer.certManager must be set"
 type PeerTLS struct {
 	// SecretRef points at a Secret in the cluster's namespace holding
 	// the peer cert+key in the standard kubernetes.io/tls shape:
@@ -203,35 +203,17 @@ type PeerTLS struct {
 	// connections — and --peer-trusted-ca-file is always populated).
 	// The peer cert MUST carry both serverAuth and clientAuth in EKU.
 	//
-	// Mutually exclusive with CertManager and AutoTLS.
+	// Mutually exclusive with CertManager.
 	// +optional
 	SecretRef *corev1.LocalObjectReference `json:"secretRef,omitempty"`
 
 	// CertManager configures operator-driven TLS material provisioning
 	// for the peer plane via cert-manager.io/v1 Certificate resources.
-	// Mutually exclusive with SecretRef and AutoTLS.
+	// Mutually exclusive with SecretRef.
 	// +optional
 	CertManager *PeerCertManagerTLS `json:"certManager,omitempty"`
-
-	// AutoTLS runs the peer API with etcd's --peer-auto-tls: each member
-	// generates its own self-signed peer certificate and there is NO shared
-	// CA, so peer traffic is encrypted but NOT authenticated — any workload
-	// that can reach :2380 and speak TLS can join or impersonate a peer.
-	// This is INSECURE and exists only to adopt legacy clusters that ran the
-	// previous operator's unconditional --peer-auto-tls default in place:
-	// etcd-migrate sets it so replacement/scaled members interoperate with
-	// the still-auto-tls adopted members without a CA rotation. Move to
-	// SecretRef or CertManager (real mTLS) when you can — that is a
-	// delete-and-recreate since spec.tls is immutable. Mutually exclusive
-	// with SecretRef and CertManager.
-	// +optional
-	AutoTLS *PeerAutoTLS `json:"autoTLS,omitempty"`
 }
 
-// PeerAutoTLS enables etcd's --peer-auto-tls for the peer plane. Empty struct:
-// its presence is the signal. INSECURE — see PeerTLS.AutoTLS.
-type PeerAutoTLS struct{}
-
 // PeerCertManagerTLS configures operator-driven TLS for the peer API.
 type PeerCertManagerTLS struct {
 	// IssuerRef is the Issuer or ClusterIssuer that signs the peer cert.

api/v1alpha2/etcdmember_types.go

@@ -45,10 +45,14 @@ type EtcdMemberTLS struct {
 	// +optional
 	PeerSecretRef *corev1.LocalObjectReference `json:"peerSecretRef,omitempty"`
 
-	// PeerAutoTLS mirrors "EtcdClusterTLS.Peer.AutoTLS is set": run the peer
-	// API with etcd's --peer-auto-tls (self-signed, no shared CA) instead of
-	// mounting a peer secret. INSECURE — peer is encrypted but NOT
-	// authenticated. Mutually exclusive with PeerSecretRef.
+	// PeerAutoTLS is operator-managed plumbing: it carries the cluster's
+	// reserved "etcd-operator.cozystack.io/peer-auto-tls" annotation down to
+	// the member so buildPod renders etcd's --peer-auto-tls (self-signed, no
+	// shared CA) instead of mounting a peer secret. INSECURE — peer is
+	// encrypted but NOT authenticated. Set only on clusters adopted from a
+	// legacy --peer-auto-tls cluster, and never together with PeerSecretRef
+	// (an explicit peer secret supersedes the annotation). Users do not set
+	// this directly; the cluster controller derives it.
 	// +optional
 	PeerAutoTLS bool `json:"peerAutoTLS,omitempty"`
 }

api/v1alpha2/zz_generated.deepcopy.go

@@ -1515,25 +1515,11 @@ spec:
                       is symmetric and there is no useful encrypt-only-no-identity mode
                       for it.
                     properties:
-                      autoTLS:
-                        description: |-
-                          AutoTLS runs the peer API with etcd's --peer-auto-tls: each member
-                          generates its own self-signed peer certificate and there is NO shared
-                          CA, so peer traffic is encrypted but NOT authenticated — any workload
-                          that can reach :2380 and speak TLS can join or impersonate a peer.
-                          This is INSECURE and exists only to adopt legacy clusters that ran the
-                          previous operator's unconditional --peer-auto-tls default in place:
-                          etcd-migrate sets it so replacement/scaled members interoperate with
-                          the still-auto-tls adopted members without a CA rotation. Move to
-                          SecretRef or CertManager (real mTLS) when you can — that is a
-                          delete-and-recreate since spec.tls is immutable. Mutually exclusive
-                          with SecretRef and CertManager.
-                        type: object
                       certManager:
                         description: |-
                           CertManager configures operator-driven TLS material provisioning
                           for the peer plane via cert-manager.io/v1 Certificate resources.
-                          Mutually exclusive with SecretRef and AutoTLS.
+                          Mutually exclusive with SecretRef.
                         properties:
                           issuerRef:
                             description: |-
@@ -1570,7 +1556,7 @@ spec:
                           connections — and --peer-trusted-ca-file is always populated).
                           The peer cert MUST carry both serverAuth and clientAuth in EKU.
 
-                          Mutually exclusive with CertManager and AutoTLS.
+                          Mutually exclusive with CertManager.
                         properties:
                           name:
                             default: ""
@@ -1585,10 +1571,9 @@ spec:
                         x-kubernetes-map-type: atomic
                     type: object
                     x-kubernetes-validations:
-                    - message: exactly one of spec.tls.peer.secretRef, spec.tls.peer.certManager
-                        or spec.tls.peer.autoTLS must be set
-                      rule: '[has(self.secretRef), has(self.certManager), has(self.autoTLS)].filter(x,
-                        x).size() == 1'
+                    - message: exactly one of spec.tls.peer.secretRef or spec.tls.peer.certManager
+                        must be set
+                      rule: has(self.secretRef) != has(self.certManager)
                 type: object
               topologySpreadConstraints:
                 description: |-

charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml

@@ -1338,10 +1338,14 @@ spec:
                     x-kubernetes-map-type: atomic
                   peerAutoTLS:
                     description: |-
-                      PeerAutoTLS mirrors "EtcdClusterTLS.Peer.AutoTLS is set": run the peer
-                      API with etcd's --peer-auto-tls (self-signed, no shared CA) instead of
-                      mounting a peer secret. INSECURE — peer is encrypted but NOT
-                      authenticated. Mutually exclusive with PeerSecretRef.
+                      PeerAutoTLS is operator-managed plumbing: it carries the cluster's
+                      reserved "etcd-operator.cozystack.io/peer-auto-tls" annotation down to
+                      the member so buildPod renders etcd's --peer-auto-tls (self-signed, no
+                      shared CA) instead of mounting a peer secret. INSECURE — peer is
+                      encrypted but NOT authenticated. Set only on clusters adopted from a
+                      legacy --peer-auto-tls cluster, and never together with PeerSecretRef
+                      (an explicit peer secret supersedes the annotation). Users do not set
+                      this directly; the cluster controller derives it.
                     type: boolean
                   peerSecretRef:
                     description: |-

charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml

@@ -221,6 +221,7 @@ func runMigration(ctx context.Context, cfg *Config, stdin io.Reader, stdout io.W
 		fmt.Fprintln(stdout, "\nNEXT: scale the new operator up — it will take over the adopted clusters without touching the pods:\n  kubectl -n "+
 			mustNamespace(cfg.NewController)+" scale deploy "+mustName(cfg.NewController)+" --replicas=1")
 	}
+	renderSecuritySummary(stdout, plans)
 	printCRDNotice(stdout)
 	return errorIfPlanFailed(plans)
 }

cmd/etcd-migrate/main.go

@@ -30,6 +30,9 @@ func render(w io.Writer, plans []migrate.ResourcePlan) {
 		for _, e := range p.Errors {
 			fmt.Fprintf(w, "  ERROR: %s\n", e)
 		}
+		for _, sw := range p.SecurityWarnings {
+			fmt.Fprintf(w, "  ⚠️  SECURITY: %s\n", sw)
+		}
 		for _, warn := range p.Warnings {
 			fmt.Fprintf(w, "  warning: %s\n", warn)
 		}
@@ -85,6 +88,29 @@ func renderManifest(w io.Writer, obj client.Object) {
 	_, _ = w.Write(data)
 }
 
+// renderSecuritySummary re-surfaces every SecurityWarning from the plans that
+// were actually adopted, AFTER --apply has run. The pre-apply plan already
+// shows them, but for a security-posture downgrade (e.g. an unauthenticated
+// --peer-auto-tls peer plane) that is not enough: the plan scrolls past, so the
+// operator must see the downgrade again in the closing summary, once it is a
+// fait accompli. No-op when nothing was downgraded.
+func renderSecuritySummary(w io.Writer, plans []migrate.ResourcePlan) {
+	var any bool
+	for i := range plans {
+		p := &plans[i]
+		if p.Action != migrate.ActionAdopt || len(p.SecurityWarnings) == 0 {
+			continue
+		}
+		if !any {
+			fmt.Fprintln(w, "\n⚠️  SECURITY — review before relying on the adopted clusters:")
+			any = true
+		}
+		for _, sw := range p.SecurityWarnings {
+			fmt.Fprintf(w, "  • %s/%s: %s\n", p.Namespace, p.SourceName, sw)
+		}
+	}
+}
+
 // printCRDNotice reminds about the one cleanup step the tool never performs.
 func printCRDNotice(w io.Writer) {
 	fmt.Fprintln(w, `

cmd/etcd-migrate/output.go

@@ -717,8 +717,9 @@ func (r *EtcdMemberReconciler) buildPod(member *lll.EtcdMember) *corev1.Pod {
 		// INSECURE legacy-compat peer mode: etcd generates a self-signed peer
 		// cert per member with no shared CA, so peer is encrypted but NOT
 		// authenticated and there is nothing to mount. Only reached for
-		// clusters adopted from a --peer-auto-tls legacy cluster (see
-		// EtcdClusterTLS.Peer.AutoTLS); etcd-migrate sets it.
+		// clusters adopted from a --peer-auto-tls legacy cluster: the cluster
+		// controller derives this from the reserved AnnPeerAutoTLS annotation
+		// etcd-migrate stamps (see AnnPeerAutoTLS).
 		cmd = append(cmd, "--peer-auto-tls")
 	case peerTLS:
 		cmd = append(cmd,

controllers/etcdmember_controller.go

@@ -2493,6 +2493,7 @@ func TestDeriveMemberTLS(t *testing.T) {
 		hasClient    bool
 		hasPeer      bool
 		clientMTLS   bool
+		peerAutoTLS  bool
 		serverSecret string
 		opSecret     string
 		peerSecret   string
@@ -2569,6 +2570,29 @@ func TestDeriveMemberTLS(t *testing.T) {
 			}}}),
 			want: want{hasPeer: true, peerSecret: "etcd-peer-tls"},
 		},
+		{
+			// Legacy-compat --peer-auto-tls carried on the reserved cluster
+			// annotation (no typed spec.tls.peer) projects to PeerAutoTLS.
+			name: "peer-auto-tls annotation only",
+			in: func() *lll.EtcdCluster {
+				c := withName(&lll.EtcdCluster{})
+				c.Annotations = map[string]string{AnnPeerAutoTLS: "true"}
+				return c
+			}(),
+			want: want{peerAutoTLS: true},
+		},
+		{
+			// An explicit peer secretRef supersedes the annotation.
+			name: "peer secretRef beats peer-auto-tls annotation",
+			in: func() *lll.EtcdCluster {
+				c := withName(&lll.EtcdCluster{Spec: lll.EtcdClusterSpec{TLS: &lll.EtcdClusterTLS{
+					Peer: &lll.PeerTLS{SecretRef: &corev1.LocalObjectReference{Name: "p"}},
+				}}})
+				c.Annotations = map[string]string{AnnPeerAutoTLS: "true"}
+				return c
+			}(),
+			want: want{hasPeer: true, peerSecret: "p"},
+		},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -2588,6 +2612,9 @@ func TestDeriveMemberTLS(t *testing.T) {
 			if (got.PeerSecretRef != nil) != tc.want.hasPeer {
 				t.Fatalf("hasPeer = %v; want %v", got.PeerSecretRef != nil, tc.want.hasPeer)
 			}
+			if got.PeerAutoTLS != tc.want.peerAutoTLS {
+				t.Fatalf("PeerAutoTLS = %v; want %v", got.PeerAutoTLS, tc.want.peerAutoTLS)
+			}
 			if got.ClientMTLS != tc.want.clientMTLS {
 				t.Fatalf("ClientMTLS = %v; want %v", got.ClientMTLS, tc.want.clientMTLS)
 			}