[helm] Add support advertised listeners for out of cluster clients - #4108
[helm] Add support advertised listeners for out of cluster clients#4108charlesdong1991 wants to merge 2 commits into
Conversation
|
can you take a look if this is something we want? @affo thanks |
affo
left a comment
There was a problem hiding this comment.
Thanks @charlesdong1991 — yes, this is something we want. The problem is real (the classic Kafka advertised-listeners issue), the default-off scoping is right, and deliberately not managing per-pod Services keeps this a proper chart-level stopgap until the k8s operator (FIP-41) owns external access. Also +1 on hard-failing when configurationOverrides sets bind.listeners/advertised.listeners — the chart and the override would silently fight otherwise.
I have three design-level asks on the values API before we lock it in (it becomes public API of the chart), plus some smaller points.
1. Per-pod uniqueness: ordinal arrays instead of shell formulas
advertisedPort: "$((9126 + ${POD_NAME##*-}))" works, but it makes "the startup command is a shell script" part of the chart's public contract, and it's not schema-checkable — a typo fails only at client-redirect time. Prefer the Bitnami approach (cf. externalAccess.*.service.nodePorts, "length must be the same as replicaCount"): per-ordinal arrays.
Accepted trade-off: static arrays can't express ${NODE_IP}. Users creating per-pod NodePorts themselves would provide statically known IPs/hostnames (Bitnami externalIPs style). If you think the NODE_IP case is important enough, we could keep a whitelisted ${NODE_IP} substitution in advertisedHost only — open to discussing.
2. Drop listeners.client.advertisedHost/advertisedPort
CLIENT (like INTERNAL) addressing should stay chart-owned: a bad override bricks in-cluster connectivity, and the failure only shows at produce/fetch time. The two use cases it covers are better served otherwise:
- Non-
cluster.localclusters (real gap — the FQDN is hardcoded today): add a top-levelclusterDomain: cluster.localvalue, as Bitnami does. - Split-horizon DNS (the one case the docs cite): covered by EXTERNAL with the split name as
advertisedHost; only the single-bootstrap-address aesthetic is lost.
Adding the override back later is a compatible change; removing ig — so let's start narrow. This also deletes the CLIENT branch ofthe replica warning and shrinks the helpers.
3. EXTERNAL auth: make it explicit, not inherited
Right now EXTERNAL silently mirrors the CLIENT protocol and JAAS users. Was that a deliberate choice or a shortcut? Fluss natively supports per-listener auth
(security.protocol.map + per-listener JAAS contexts, <listenertext), and every comparable system (Kafka broker per-listenerconfigs, Bitnami listeners.external.protocol, Strimzi per-listener tls/auth) makes external security a first-class, per-listener setting. Proposal: a
security.external.sasl block symmetric with security.client.sa plain | client — where client is the explicit opt-in totoday's mirroring behavior. Two safety nets regardless of shape:
VALUES WARNINGwhenexternal.enabledand the external listener resolves to PLAINTEXT — an unauthenticated listener exposed outside the cluster should
never be silent.- A docs warning that SASL/PLAIN without TLS sends credentials in cleartext; until TLS lands (FIP-29), external exposure should be limited to trusted
networks.
Resulting values.yaml
clusterDomain: cluster.local
listeners:
internal:
port: 9123
client:
port: 9124 # advertised address stays chart-owned (pod FQDN)
external:
enabled: false
port: 9125
advertisedHost: "" # shared default host, e.g. "fluss.example.com"
advertisedPort: "" # shared default port (needs per-pod hosts)
coordinator:
listeners:
external:
advertisedHost: ""
advertisedPort: ""
tablet:
listeners:
external:
advertisedHost: "" # shared host for all tablets, or:
advertisedHosts: [] # per-ordinal; length == tablet.numberOfReplicas
advertisedPorts: [] # per-ordinal; length == tablet.numberOfReplicas
security:
external:
sasl:
mechanism: "" # "" (PLAINTEXT + warning) | plain | c
plain:
users: [] # same shape as security.client.sasl.pThe port-forward example from the docs then becomes:
listeners:
external:
enabled: true
coordinator:
listeners:
external:
advertisedHost: "127.0.0.1"
advertisedPort: "9125"
tablet:
listeners:
external:
advertisedHost: "127.0.0.1"
advertisedPorts: [9126, 9127, 9128]Smaller points
- Drop
listeners.external.nameand hardcodeEXTERNAL: nothing validates the name survivesserver.yaml/JAAS parsing, the k8s port names stay hardcoxternalanyway, and it can collide withconfigurationOverride]`. - The external-port collision check should also cover
metrics.port(9249/9250), otherwise you get a duplicate container port. The per-component override keys are typo-sensitive with no `val array design above a schema becomes easy to add — fine as afollow-up.
Happy to help iterate — the helpers/tests/docs structure is solid, this is only about the values surface.
Purpose
Linked issue: close #4092
There is currently no way to run a Fluss client from outside the k8s cluster. Not sure if it is something we want, but I got this issue when trying to set up in our env...
I can close if it is not something we need.