fix: publish service DNS name instead of ClusterIP in connectionString

[WentingWu666666]

Summary

When spec.exposeViaService.serviceType is ClusterIP, status.connectionString published the raw ClusterIP (e.g. 10.96.x.x) as the host. This PR publishes the in-cluster DNS name (<service>.<namespace>.svc) instead.

Fixes #201.

Why not localhost?

The original issue proposed substituting localhost. While investigating I found that would be wrong for in-cluster consumers (I raised this as an open question on the issue back in June). More importantly, the investigation surfaced that the raw ClusterIP is not merely cosmetic — it is broken under strict TLS, which makes the DNS name the correct fix for both audiences and resolves the open question.

Root cause

The gateway certificate created by certificate_controller.go carries DNS SANs only — no IP SAN:

serviceBase := util.DOCUMENTDB_SERVICE_PREFIX + ddb.Name
dnsNames := []string{
    serviceBase,
    serviceBase + "." + namespace,
    serviceBase + "." + namespace + ".svc",
}

Meanwhile GenerateConnectionString always sets tls=true, and omits tlsAllowInvalidCertificates=true exactly when status.tls.Ready is true:

if !trustTLS {
    conn += "&tlsAllowInvalidCertificates=true"
}

So once TLS is trusted, a client following the published string performs strict hostname verification against an IP address that has no matching SAN, and the connection fails. The failure was masked on the untrusted path because tlsAllowInvalidCertificates=true skips verification entirely.

The fix

One-line behavioral change in EnsureServiceIP: for ClusterIP services it now returns <service>.<namespace>.svc. That address:

• matches an existing certificate SAN, so strict TLS verification succeeds;
• resolves from any pod in the cluster (the in-cluster consumer case);
• survives Service recreation, which reassigns the ClusterIP.

The name is read from the Service object rather than recomputed from the CR name, so the 63-character truncation applied in GetDocumentDBServiceDefinition is preserved.

LoadBalancer behavior is unchanged — the external ingress IP/hostname is still returned, since cluster-internal DNS is not resolvable by outside clients.

No call-site changes are needed: GenerateConnectionString already accepts an arbitrary host (it has existing hostname and IPv6 test cases), so the diff is confined to internal/utils.

Tests

• TestEnsureServiceIP extended: asserts the exact DNS name for ClusterIP. The previous success case only asserted non-empty, which is precisely why this regression was invisible. Added LoadBalancer IP and hostname cases alongside the existing error cases.
• go build ./..., go vet ./..., gofmt clean.
• Full operator suite green: api/preview, internal/cnpg, internal/controller, internal/otel, internal/utils, internal/webhook.
• Diff coverage reproduced locally with the same go-patch-cover gate CI uses: 100% (4/4 changed statements), against the 90% threshold.

e2e compatibility

test/e2e/tests/status/connection_string_test.go is unaffected: it asserts only that the host is non-empty and that the port matches the gateway port, and it dials via port-forward rather than the published host. Its splitHostPort scans from the right for :, so a dotted DNS name parses correctly.

Notes for reviewers

Two adjacent items I deliberately left out of scope:

1. EnsureServiceIP no longer always returns an IP. I initially renamed it to EnsureServiceEndpoint, but that pulled documentdb_controller.go into the diff — adding churn on behaviorally-unchanged lines, overlapping with in-flight feat: fail-fast preflight for schema upgrade path (multi-minor jumps) #444, and dragging patch coverage down to 43% on Reconcile lines that unit tests do not reach. I reverted the rename and documented the behavior on the function instead. Happy to do the rename as a standalone follow-up once feat: fail-fast preflight for schema upgrade path (multi-minor jumps) #444 lands.

2. Pre-existing: the cert SANs are built from DOCUMENTDB_SERVICE_PREFIX + ddb.Name without the 63-char truncation that GetDocumentDBServiceDefinition applies to the real Service name. For a sufficiently long CR name the SAN and the actual service name would diverge. Not touched here — can file a follow-up if useful.

[Copilot]

Pull request overview

Updates status.connectionString to publish stable Service DNS names for ClusterIP services while preserving LoadBalancer endpoints.

Changes:

• Renames endpoint resolution to EnsureServiceEndpoint.
• Adds ClusterIP DNS and LoadBalancer endpoint test coverage.
• Updates reconciliation to use the resolved endpoint.
• Critical finding: certificate SANs do not account for 63-character Service-name truncation, so strict TLS can still fail for long names.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Summary
operator/src/internal/utils/util.go	Resolves ClusterIP services to DNS endpoints.
operator/src/internal/utils/util_test.go	Tests DNS, IP, hostname, and error cases.
operator/src/internal/controller/documentdb_controller.go	Publishes the resolved endpoint in connection strings.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[documentdb-triage-tool]

🤖 Auto-triaged by documentdb-triage-tool.

Applied: go, test, bug, performance
Project fields suggested: Component controllers · Priority P1 · Effort M · Status Needs Review
Confidence: 0.88 (mixed)

Reasoning

component from path globs (controllers, test); effort from diff stats (71+21 LOC, 3 files); LLM: Fixes a broken connection string under strict TLS when ClusterIP is published instead of a DNS SAN — an active functional blocker for TLS-enabled in-cluster consumers, with a focused rename + logic change in one component and extended tests.

If a label is wrong, remove it manually and ping @patty-chow so the rules can be tuned. The bot will not re-label items that already have component labels.

[WentingWu666666]

Pushed e482849 addressing the review thread on the 63-character truncation: the Service name and the gateway cert SANs now both derive from a single util.GetDocumentDBServiceName helper, so they can't drift for long DocumentDB names. Added regression coverage that fails on the previous commit. Ready for another look @xgerman @alaye-ms @hossain-rayhan.