From 65f1aea1a3b5f93564eadef4db013830efe289e4 Mon Sep 17 00:00:00 2001 From: Vimal Solanki Date: Mon, 11 May 2026 20:00:25 +0530 Subject: [PATCH 1/2] fix(cpo/pki): add unit tests for etcd peer certificate SANs Add unit tests for ReconcileEtcdPeerSecret to verify the peer certificate includes correct etcd-discovery SANs and key usages. Include a comment explaining why etcd-client SANs are not needed: the etcd-client service uses ClusterIP (not headless), so it does not create per-pod PTR records that would cause reverse DNS ambiguity during peer TLS verification. Signed-off-by: Vimal Solanki --- .../hostedcontrolplane/pki/etcd.go | 3 + .../hostedcontrolplane/pki/etcd_test.go | 81 +++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 control-plane-operator/controllers/hostedcontrolplane/pki/etcd_test.go diff --git a/control-plane-operator/controllers/hostedcontrolplane/pki/etcd.go b/control-plane-operator/controllers/hostedcontrolplane/pki/etcd.go index 69c6526d7b64..9ce0a4905802 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/pki/etcd.go +++ b/control-plane-operator/controllers/hostedcontrolplane/pki/etcd.go @@ -45,6 +45,9 @@ func ReconcileEtcdPeerSecret(secret, ca *corev1.Secret, ownerRef config.OwnerRef dnsNames := []string{ fmt.Sprintf("*.etcd-discovery.%s.svc", secret.Namespace), fmt.Sprintf("*.etcd-discovery.%s.svc.cluster.local", secret.Namespace), + // etcd-client uses a ClusterIP service (not headless), so it does not create per-pod + // PTR records and cannot cause reverse DNS ambiguity during peer TLS verification. + // TODO(OCPBUGS-86648): move IPs to the ips parameter of reconcileSignedCertWithKeysAndAddresses. "127.0.0.1", "::1", } diff --git a/control-plane-operator/controllers/hostedcontrolplane/pki/etcd_test.go b/control-plane-operator/controllers/hostedcontrolplane/pki/etcd_test.go new file mode 100644 index 000000000000..5a60b415ed6a --- /dev/null +++ b/control-plane-operator/controllers/hostedcontrolplane/pki/etcd_test.go @@ -0,0 +1,81 @@ +package pki + +import ( + "crypto/x509" + "crypto/x509/pkix" + "testing" + + . "github.com/onsi/gomega" + + "github.com/openshift/hypershift/support/certs" + "github.com/openshift/hypershift/support/config" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestReconcileEtcdPeerSecret(t *testing.T) { + t.Parallel() + + caCfg := certs.CertCfg{ + IsCA: true, + Subject: pkix.Name{CommonName: "etcd-signer", OrganizationalUnit: []string{"openshift"}}, + } + caKey, caCert, err := certs.GenerateSelfSignedCertificate(&caCfg) + if err != nil { + t.Fatalf("failed to generate CA: %v", err) + } + caSecret := &corev1.Secret{ + Data: map[string][]byte{ + certs.CASignerCertMapKey: certs.CertToPem(caCert), + certs.CASignerKeyMapKey: certs.PrivateKeyToPem(caKey), + }, + } + + t.Run("When reconciling etcd peer secret it should include etcd-discovery SANs", func(t *testing.T) { + g := NewWithT(t) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "clusters-test", + }, + } + + err := ReconcileEtcdPeerSecret(secret, caSecret, config.OwnerRef{}) + g.Expect(err).ToNot(HaveOccurred()) + + certData := secret.Data[EtcdPeerCrtKey] + g.Expect(certData).ToNot(BeEmpty()) + + cert, err := certs.PemToCertificate(certData) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(cert.DNSNames).To(ContainElements( + "*.etcd-discovery.clusters-test.svc", + "*.etcd-discovery.clusters-test.svc.cluster.local", + // TODO(OCPBUGS-86648): assert on cert.IPAddresses instead once IPs are moved out of dnsNames. + "127.0.0.1", + "::1", + )) + g.Expect(cert.DNSNames).ToNot(ContainElement(ContainSubstring("etcd-client"))) + }) + + t.Run("When reconciling etcd peer secret it should have client and server auth usage", func(t *testing.T) { + g := NewWithT(t) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "clusters-test", + }, + } + + err := ReconcileEtcdPeerSecret(secret, caSecret, config.OwnerRef{}) + g.Expect(err).ToNot(HaveOccurred()) + + cert, err := certs.PemToCertificate(secret.Data[EtcdPeerCrtKey]) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(cert.ExtKeyUsage).To(ContainElements( + x509.ExtKeyUsageClientAuth, + x509.ExtKeyUsageServerAuth, + )) + }) +} From 9a5a0f8735e1301e27a46e3da1b638cf5d66f21c Mon Sep 17 00:00:00 2001 From: Vimal Solanki Date: Fri, 29 May 2026 06:36:43 +0530 Subject: [PATCH 2/2] fix(cpo/etcd): switch etcd-client to ClusterIP service Both etcd-client and etcd-discovery headless services select the same etcd pods, causing dual PTR records per pod IP. Converting etcd-client to a regular ClusterIP service eliminates the duplicate PTR records at the root cause, complementing the SAN-based fix. Co-Authored-By: Cesar Wong Signed-off-by: Vimal Solanki --- ...z_fixture_TestControlPlaneComponents_etcd_client_service.yaml | 1 - ...z_fixture_TestControlPlaneComponents_etcd_client_service.yaml | 1 - ...z_fixture_TestControlPlaneComponents_etcd_client_service.yaml | 1 - ...z_fixture_TestControlPlaneComponents_etcd_client_service.yaml | 1 - ...z_fixture_TestControlPlaneComponents_etcd_client_service.yaml | 1 - .../controllers/hostedcontrolplane/v2/assets/etcd/service.yaml | 1 - 6 files changed, 6 deletions(-) diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/AROSwift/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/AROSwift/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml index 9281169c4e4c..5b38919b362b 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/AROSwift/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/AROSwift/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml @@ -14,7 +14,6 @@ metadata: uid: "" resourceVersion: "1" spec: - clusterIP: None ports: - name: etcd-client port: 2379 diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/GCP/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/GCP/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml index 9281169c4e4c..5b38919b362b 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/GCP/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/GCP/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml @@ -14,7 +14,6 @@ metadata: uid: "" resourceVersion: "1" spec: - clusterIP: None ports: - name: etcd-client port: 2379 diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/IBMCloud/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/IBMCloud/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml index 9281169c4e4c..5b38919b362b 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/IBMCloud/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/IBMCloud/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml @@ -14,7 +14,6 @@ metadata: uid: "" resourceVersion: "1" spec: - clusterIP: None ports: - name: etcd-client port: 2379 diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml index 9281169c4e4c..5b38919b362b 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml @@ -14,7 +14,6 @@ metadata: uid: "" resourceVersion: "1" spec: - clusterIP: None ports: - name: etcd-client port: 2379 diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml index 9281169c4e4c..5b38919b362b 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/etcd/zz_fixture_TestControlPlaneComponents_etcd_client_service.yaml @@ -14,7 +14,6 @@ metadata: uid: "" resourceVersion: "1" spec: - clusterIP: None ports: - name: etcd-client port: 2379 diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/assets/etcd/service.yaml b/control-plane-operator/controllers/hostedcontrolplane/v2/assets/etcd/service.yaml index 1eda5d8ddffd..b2c7abe18014 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/assets/etcd/service.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/assets/etcd/service.yaml @@ -5,7 +5,6 @@ metadata: app: etcd name: etcd-client spec: - clusterIP: None ports: - name: etcd-client port: 2379