Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions operator/src/internal/controller/certificate_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package controller

import (
"context"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down
41 changes: 33 additions & 8 deletions operator/src/internal/utils/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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{
Expand Down Expand Up @@ -107,16 +120,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 (<service>.<namespace>.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
Comment thread
WentingWu666666 marked this conversation as resolved.
}
return "", fmt.Errorf("ClusterIP not assigned")
}
Expand Down
100 changes: 92 additions & 8 deletions operator/src/internal/utils/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -696,12 +696,57 @@ 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
service *corev1.Service
expectError bool
errorMsg string
name string
service *corev1.Service
expectError bool
errorMsg string
expectedResult string
}{
{
name: "nil service returns error",
Expand All @@ -710,14 +755,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",
Expand All @@ -741,6 +791,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 {
Expand All @@ -758,8 +842,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)
}
}
})
Expand Down
7 changes: 5 additions & 2 deletions test/e2e/pkg/e2eutils/portforward/portforward.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 ""
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/pkg/e2eutils/portforward/portforward_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading