From 3816ae75241a85de31fd9869060da3f8f67252f6 Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Sun, 23 Aug 2026 09:07:12 -0400 Subject: [PATCH 1/2] fix: publish service DNS name instead of ClusterIP in connectionString When spec.exposeViaService.serviceType is ClusterIP, status.connectionString embedded the raw ClusterIP (e.g. 10.96.x.x) as the host. That address is routable in-cluster, but it breaks TLS hostname verification: the gateway certificate issued by the certificate controller carries only DNS SANs (documentdb-service-, ., and ..svc) and no IP SAN. Once status.tls.Ready is true the connection string omits tlsAllowInvalidCertificates, so a client following the published string performs strict verification against an IP with no matching SAN and fails. Return the in-cluster DNS name ..svc for ClusterIP services instead. It matches an existing certificate SAN, resolves from any pod in the cluster, and stays valid when the Service is recreated with a different ClusterIP. The name is read from the Service object so the 63-char truncation applied when the Service is built is preserved. LoadBalancer behavior is unchanged: the external ingress IP/hostname is still returned, since cluster-internal DNS is not resolvable by outside clients. EnsureServiceIP is renamed to EnsureServiceEndpoint since it no longer returns an IP in every case. Fixes #201 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- operator/src/internal/utils/util.go | 18 ++++++-- operator/src/internal/utils/util_test.go | 56 ++++++++++++++++++++---- 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/operator/src/internal/utils/util.go b/operator/src/internal/utils/util.go index 10511696b..db34c183d 100644 --- a/operator/src/internal/utils/util.go +++ b/operator/src/internal/utils/util.go @@ -107,16 +107,28 @@ func getEnvironmentSpecificAnnotations(environment string) map[string]string { } } -// EnsureServiceIP ensures that the Service has an IP assigned and returns it, or returns an error if not available +// EnsureServiceIP ensures that the Service has a reachable endpoint and returns +// the address clients should connect to, or an error if it is not available yet. +// +// For ClusterIP services this returns the in-cluster DNS name (..svc) +// rather than the raw ClusterIP. The DNS name resolves from any pod in the cluster, stays +// valid when the Service is recreated with a different ClusterIP, and matches the SANs on +// the gateway certificate, so strict TLS verification succeeds. A raw ClusterIP has no +// matching SAN and fails hostname verification once TLS is trusted. +// +// For LoadBalancer services the externally reachable ingress IP/hostname is returned, +// since cluster-internal DNS is not resolvable by outside clients. +// +// NOTE: the returned value is therefore not always an IP address, despite the name. func EnsureServiceIP(ctx context.Context, service *corev1.Service) (string, error) { if service == nil { return "", fmt.Errorf("service is nil") } - // For ClusterIP services, return the ClusterIP directly + // For ClusterIP services, return the stable in-cluster DNS name if service.Spec.Type == corev1.ServiceTypeClusterIP { if service.Spec.ClusterIP != "" && service.Spec.ClusterIP != "None" { - return service.Spec.ClusterIP, nil + return fmt.Sprintf("%s.%s.svc", service.Name, service.Namespace), nil } return "", fmt.Errorf("ClusterIP not assigned") } diff --git a/operator/src/internal/utils/util_test.go b/operator/src/internal/utils/util_test.go index b0977add2..66eb68a26 100644 --- a/operator/src/internal/utils/util_test.go +++ b/operator/src/internal/utils/util_test.go @@ -698,10 +698,11 @@ func TestGenerateServiceName_PublicFunction(t *testing.T) { func TestEnsureServiceIP(t *testing.T) { tests := []struct { - name string - service *corev1.Service - expectError bool - errorMsg string + name string + service *corev1.Service + expectError bool + errorMsg string + expectedResult string }{ { name: "nil service returns error", @@ -710,14 +711,19 @@ func TestEnsureServiceIP(t *testing.T) { errorMsg: "service is nil", }, { - name: "ClusterIP service with valid IP", + name: "ClusterIP service returns in-cluster DNS name, not the raw IP", service: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "documentdb-service-test-db", + Namespace: "test-namespace", + }, Spec: corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, ClusterIP: "10.0.0.1", }, }, - expectError: false, + expectError: false, + expectedResult: "documentdb-service-test-db.test-namespace.svc", }, { name: "ClusterIP service with None returns error", @@ -741,6 +747,40 @@ func TestEnsureServiceIP(t *testing.T) { expectError: true, errorMsg: "ClusterIP not assigned", }, + { + name: "LoadBalancer service returns external IP", + service: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "documentdb-service-test-db", + Namespace: "test-namespace", + }, + Spec: corev1.ServiceSpec{Type: corev1.ServiceTypeLoadBalancer}, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{{IP: "203.0.113.10"}}, + }, + }, + }, + expectError: false, + expectedResult: "203.0.113.10", + }, + { + name: "LoadBalancer service returns external hostname", + service: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "documentdb-service-test-db", + Namespace: "test-namespace", + }, + Spec: corev1.ServiceSpec{Type: corev1.ServiceTypeLoadBalancer}, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{{Hostname: "lb.example.com"}}, + }, + }, + }, + expectError: false, + expectedResult: "lb.example.com", + }, } for _, tt := range tests { @@ -758,8 +798,8 @@ func TestEnsureServiceIP(t *testing.T) { if err != nil { t.Errorf("Unexpected error: %v", err) } - if result == "" { - t.Error("Expected non-empty result") + if result != tt.expectedResult { + t.Errorf("EnsureServiceIP() = %q; expected %q", result, tt.expectedResult) } } }) From e482849191ed46863ba9b8a94ed4c7266790f353 Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Mon, 24 Aug 2026 09:07:27 -0400 Subject: [PATCH 2/2] fix: derive gateway cert SANs from the truncated service name GetDocumentDBServiceDefinition truncates the Service name to the 63-character RFC 1123 limit, but certificate_controller.go built the gateway cert SANs from the untruncated DOCUMENTDB_SERVICE_PREFIX + ddb.Name. For DocumentDB names longer than 44 characters the two diverged, so the DNS name now published in status.connectionString had no matching SAN and failed strict TLS verification. Introduce util.GetDocumentDBServiceName as the single source of truth for that name and use it in the Service definition, both cert SAN call sites, and the e2e port-forward helper that mirrors it. Also trim a trailing separator left behind by truncation, which would otherwise produce an invalid RFC 1123 label the API server rejects. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- .../controller/certificate_controller.go | 4 +- .../controller/certificate_controller_test.go | 31 ++++++++++++- operator/src/internal/utils/util.go | 23 +++++++--- operator/src/internal/utils/util_test.go | 44 +++++++++++++++++++ .../pkg/e2eutils/portforward/portforward.go | 7 ++- .../e2eutils/portforward/portforward_test.go | 5 +++ 6 files changed, 104 insertions(+), 10 deletions(-) diff --git a/operator/src/internal/controller/certificate_controller.go b/operator/src/internal/controller/certificate_controller.go index 3335f986d..a88b608d1 100644 --- a/operator/src/internal/controller/certificate_controller.go +++ b/operator/src/internal/controller/certificate_controller.go @@ -189,7 +189,7 @@ func (r *CertificateReconciler) ensureCertManagerManagedCert(ctx context.Context secretName = ddb.Name + "-gateway-cert-tls" } - serviceBase := util.DOCUMENTDB_SERVICE_PREFIX + ddb.Name + serviceBase := util.GetDocumentDBServiceName(ddb.Name) baseDNS := []string{serviceBase, serviceBase + "." + ddb.Namespace, serviceBase + "." + ddb.Namespace + ".svc"} dnsSet := map[string]struct{}{} finalDNS := []string{} @@ -289,7 +289,7 @@ func (r *CertificateReconciler) ensureSelfSignedCert(ctx context.Context, ddb *d } } - serviceBase := util.DOCUMENTDB_SERVICE_PREFIX + ddb.Name + serviceBase := util.GetDocumentDBServiceName(ddb.Name) dnsNames := []string{ serviceBase, serviceBase + "." + namespace, diff --git a/operator/src/internal/controller/certificate_controller_test.go b/operator/src/internal/controller/certificate_controller_test.go index 9753fc249..c0ac228d8 100644 --- a/operator/src/internal/controller/certificate_controller_test.go +++ b/operator/src/internal/controller/certificate_controller_test.go @@ -5,6 +5,7 @@ package controller import ( "context" + "strings" "testing" "time" @@ -126,7 +127,7 @@ func TestEnsureCertManagerManagedCert(t *testing.T) { } require.Contains(t, cert.Spec.DNSNames, "custom.example") // Should include service DNS names - serviceBase := util.DOCUMENTDB_SERVICE_PREFIX + ddb.Name + serviceBase := util.GetDocumentDBServiceName(ddb.Name) require.Contains(t, cert.Spec.DNSNames, serviceBase) // Simulate readiness condition then invoke ensure again (mimic reconcile loop) @@ -165,6 +166,34 @@ func TestEnsureSelfSignedCert(t *testing.T) { require.NotEmpty(t, ddb.Status.TLS.SecretName) } +func TestGatewayCertSANsMatchTruncatedServiceName(t *testing.T) { + ctx := context.Background() + // 19-char prefix + this name exceeds the 63-character Service name limit, so the + // operator truncates the Service name. The cert SANs must follow that truncation, + // otherwise the hostname published in status.connectionString fails strict TLS + // verification. Regression test for the SAN/Service-name divergence. + longName := strings.Repeat("a", 60) + ddb := baseDocumentDB(longName, "default") + ddb.Spec.TLS = &dbpreview.TLSConfiguration{Gateway: &dbpreview.GatewayTLS{Mode: "SelfSigned"}} + ddb.Status.TLS = &dbpreview.TLSStatus{} + r := buildCertificateReconciler(t, ddb) + + _, err := r.reconcileCertificates(ctx, ddb) + require.NoError(t, err) + + cert := &cmapi.Certificate{} + require.NoError(t, r.Client.Get(ctx, types.NamespacedName{Name: longName + "-gateway-cert", Namespace: "default"}, cert)) + + serviceName := util.GetDocumentDBServiceName(ddb.Name) + require.Len(t, serviceName, 63, "test fixture should exercise the truncation path") + + require.Contains(t, cert.Spec.DNSNames, serviceName) + require.Contains(t, cert.Spec.DNSNames, serviceName+".default") + require.Contains(t, cert.Spec.DNSNames, serviceName+".default.svc") + require.NotContains(t, cert.Spec.DNSNames, util.DOCUMENTDB_SERVICE_PREFIX+ddb.Name, + "cert must not carry a SAN for the untruncated service name") +} + func TestReconcileCertificatesDoesNotManagePostgresCertificates(t *testing.T) { ctx := context.Background() ddb := baseDocumentDB("ddb-pg", "default") diff --git a/operator/src/internal/utils/util.go b/operator/src/internal/utils/util.go index db34c183d..798ce79f9 100644 --- a/operator/src/internal/utils/util.go +++ b/operator/src/internal/utils/util.go @@ -28,6 +28,23 @@ import ( dbpreview "github.com/documentdb/documentdb-operator/api/preview" ) +// GetDocumentDBServiceName returns the name of the Service published for a DocumentDB +// instance, truncated to the 63-character RFC 1123 label limit enforced by Kubernetes. +// +// This is the single source of truth for that name. Anything that has to agree with the +// Service name — most importantly the gateway certificate SANs, which must match the +// hostname published in status.connectionString for strict TLS verification to succeed — +// must derive it from here rather than concatenating the prefix and the DocumentDB name +// itself, otherwise long names silently diverge. +func GetDocumentDBServiceName(documentDBName string) string { + serviceName := DOCUMENTDB_SERVICE_PREFIX + documentDBName + if len(serviceName) > 63 { + serviceName = serviceName[:63] + } + // A label may not end in a non-alphanumeric character, which truncation can produce. + return strings.TrimRight(serviceName, "-.") +} + // GetDocumentDBServiceDefinition returns the LoadBalancer Service definition for a given DocumentDB instance func GetDocumentDBServiceDefinition(documentdb *dbpreview.DocumentDB, replicationContext *ReplicationContext, namespace string, serviceType corev1.ServiceType) *corev1.Service { // If no local HA, these two should be empty @@ -41,11 +58,7 @@ func GetDocumentDBServiceDefinition(documentdb *dbpreview.DocumentDB, replicatio } } - // Ensure service name doesn't exceed 63 characters (Kubernetes limit) - serviceName := DOCUMENTDB_SERVICE_PREFIX + documentdb.Name - if len(serviceName) > 63 { - serviceName = serviceName[:63] - } + serviceName := GetDocumentDBServiceName(documentdb.Name) service := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ diff --git a/operator/src/internal/utils/util_test.go b/operator/src/internal/utils/util_test.go index 66eb68a26..ab01543e6 100644 --- a/operator/src/internal/utils/util_test.go +++ b/operator/src/internal/utils/util_test.go @@ -696,6 +696,50 @@ func TestGenerateServiceName_PublicFunction(t *testing.T) { } } +func TestGetDocumentDBServiceName(t *testing.T) { + tests := []struct { + name string + documentDBName string + expected string + }{ + { + name: "short name is prefixed verbatim", + documentDBName: "test-db", + expected: "documentdb-service-test-db", + }, + { + name: "name at the limit is not truncated", + documentDBName: strings.Repeat("a", 44), + expected: "documentdb-service-" + strings.Repeat("a", 44), + }, + { + name: "long name is truncated to 63 characters", + documentDBName: strings.Repeat("a", 80), + expected: "documentdb-service-" + strings.Repeat("a", 44), + }, + { + name: "truncation does not leave a trailing hyphen", + documentDBName: strings.Repeat("a", 43) + "-" + strings.Repeat("b", 20), + expected: "documentdb-service-" + strings.Repeat("a", 43), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := GetDocumentDBServiceName(tt.documentDBName) + if result != tt.expected { + t.Errorf("GetDocumentDBServiceName(%q) = %q; expected %q", tt.documentDBName, result, tt.expected) + } + if len(result) > 63 { + t.Errorf("GetDocumentDBServiceName(%q) returned %d characters, exceeding the 63-character limit", tt.documentDBName, len(result)) + } + if strings.HasSuffix(result, "-") || strings.HasSuffix(result, ".") { + t.Errorf("GetDocumentDBServiceName(%q) = %q, which is not a valid RFC 1123 label", tt.documentDBName, result) + } + }) + } +} + func TestEnsureServiceIP(t *testing.T) { tests := []struct { name string diff --git a/test/e2e/pkg/e2eutils/portforward/portforward.go b/test/e2e/pkg/e2eutils/portforward/portforward.go index f0323dfaa..3cabc2458 100644 --- a/test/e2e/pkg/e2eutils/portforward/portforward.go +++ b/test/e2e/pkg/e2eutils/portforward/portforward.go @@ -23,6 +23,7 @@ import ( "errors" "fmt" "io" + "strings" "github.com/cloudnative-pg/cloudnative-pg/tests/utils/environment" "github.com/cloudnative-pg/cloudnative-pg/tests/utils/forwardconnection" @@ -42,7 +43,9 @@ const GatewayPort = 10260 const ServiceNamePrefix = "documentdb-service-" // GatewayServiceName returns the Service name the operator creates for -// the given DocumentDB CR. +// the given DocumentDB CR. It must stay in sync with +// util.GetDocumentDBServiceName in the operator, including the trailing +// separator trim that keeps a truncated name a valid RFC 1123 label. func GatewayServiceName(dd *previewv1.DocumentDB) string { if dd == nil { return "" @@ -51,7 +54,7 @@ func GatewayServiceName(dd *previewv1.DocumentDB) string { if len(name) > 63 { name = name[:63] } - return name + return strings.TrimRight(name, "-.") } // OpenWithErr establishes a port-forward from localPort on the caller's diff --git a/test/e2e/pkg/e2eutils/portforward/portforward_test.go b/test/e2e/pkg/e2eutils/portforward/portforward_test.go index 8eaa78e71..b57ea6d6b 100644 --- a/test/e2e/pkg/e2eutils/portforward/portforward_test.go +++ b/test/e2e/pkg/e2eutils/portforward/portforward_test.go @@ -30,6 +30,11 @@ func TestGatewayServiceName(t *testing.T) { // 19 (prefix) + 44 xs = 63 "documentdb-service-" + strings.Repeat("x", 44), }, + { + "truncation trims a trailing hyphen", + &previewv1.DocumentDB{ObjectMeta: metav1.ObjectMeta{Name: strings.Repeat("x", 43) + "-" + strings.Repeat("y", 20)}}, + "documentdb-service-" + strings.Repeat("x", 43), + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) {