Repository navigation
Certificates SANs FQDN - #713
Conversation
lhotari
left a comment
There was a problem hiding this comment.
Thanks for this. To set the bar I'm reviewing against, from my reply on #712:
wildcard certs are required for broker, bookie & zk TLS when hostname verification is enabled on the client side (it should be enabled for security reason). The "client" can be a pulsar proxy or broker (towards bk, zk).
an alternative for wildcard certs would be to list all possible hostnames for SAN.
fqdn mode is exactly that alternative, and it's the right shape for the issuer-forbids-wildcards case in #712 — where wildcard isn't a fallback the user can keep. So the question this review has to answer is narrow: does fqdn mode actually list all possible hostnames?
Today it doesn't, in three cases, and two of them hit broker and bookie — two of the three components that need it.
| Component | Headless svc? | Per-pod FQDN resolves? | SAN list complete? |
|---|---|---|---|
| zookeeper | yes | yes | yes — replicaCount is authoritative, no HPA |
| bookie | yes | yes | no — breaks on kubectl scale |
| broker | yes | yes | no — breaks under its own HPA |
| proxy | no (ClusterIP) | no | n/a — client, not a verified server |
What's solid
wildcard mode renders byte-identical to master across proxy/broker/bookie/recovery/toolset/zookeeper — I diffed it, so existing users are genuinely unaffected. An invalid sanMode fails the render with a clear message rather than silently emitting a certificate with no SANs, which is the right call. clusterDomain is respected throughout rather than hardcoding cluster.local.
Render recipe I used:
helm dependency build charts/pulsar
helm template test charts/pulsar --set "certs.internal_issuer.enabled=true,\
components.proxy=true,components.toolset=true,tls.enabled=true,tls.broker.enabled=true,\
tls.proxy.enabled=true,tls.zookeeper.enabled=true,tls.bookie.enabled=true,\
tls.autorecovery.enabled=true,tls.toolset.enabled=true" \
--set tls.common.sanMode=fqdn -s templates/tls-certs-internal.yaml
One more thing, not a blocker
Switching wildcard → fqdn changes Certificate.spec.dnsNames (so cert-manager reissues) but doesn't change any pod-template checksum — I rendered broker/ZK StatefulSets under both modes and got identical hashes. So Helm performs no restart and the tightened SANs may not take effect until processes reload. Not an outage on transition, since the old wildcard still covers existing ordinals while issuance completes, but worth a docs note that a rolling restart is needed — consistent with the existing TLS upgrade guidance.
Unrelated pre-existing bug I noticed while checking this
standalone-deployment.yaml sets subdomain: <fullname>-standalone-headless and advertises $(hostname -f), so its real name is <pod>.<fullname>-standalone-headless.<ns>.svc.<domain>. No SAN mode on master includes that — not even wildcard, which emits *.<fullname>-standalone…, a different subdomain. With hostname verification on, lookup-directed standalone connections would fail today. I'll file that separately; nothing for you to do here.
Reviewed with Codex gpt-5.6-sol and Claude Opus 5; every finding reproduced locally by rendering the chart.
lhotari
left a comment
There was a problem hiding this comment.
Thanks for the quick follow-up. The autoscaling, renamed-component and proxy points are all handled, and the new .ci/test-certificate-san-modes.sh is a welcome addition. Wildcard-mode output is byte-for-byte unchanged versus the base for every component, which is what I was hoping for. One correctness issue remains in fqdn mode for the function worker (inline comment); there is a smaller related gap for standalone.
Standalone: the pod's advertised name is standalone.<release>-pulsar-standalone-headless.<ns>.svc.<domain> (hostname -f, see standalone-deployment.yaml:87-90 and :158). In fqdn mode that name is not in the certificate (standalone has no replicaCount, so no per-pod SAN is emitted, and the headless name is only added for broker/zookeeper). Adding the headless service name, or the <component>.<headless> name, for standalone would cover it.
In fqdn mode, the function worker per-pod SANs used the ClusterIP service name, but the function worker StatefulSet publishes pod DNS records under its headless service. The standalone pod's advertised name (<component>.<standalone headless service>) wasn't included at all. Build the per-pod names from the headless service for function_worker and standalone, and add the standalone pod's FQDN in fqdn mode. Wildcard and none mode output is unchanged.
|
I merged
Wildcard and |
Fixes #712
Motivation
Currently certificates are generated with wildcard SANs.
This PR allow to generate SANs with either :
tls.<component>.dnsNames)Modifications
Add
tls.common.sanMode:wildcardmode :fqdnmode :nonemode :Verifying this change