feat: support batch TCP/UDP port ranges in stream config#264
Open
feat: support batch TCP/UDP port ranges in stream config#264
Conversation
Add support for port range notation and comma-separated port lists
in gateway.stream.tcp and gateway.stream.udp configuration.
ConfigMap passes range strings through to config.yaml where the
gateway's native parsing handles them. For K8s container ports and
service ports, new helper templates (gateway.expandPorts,
gateway.normalizedTcpPorts, gateway.normalizedUdpPorts) expand
port ranges into individual port entries since Kubernetes requires
concrete port numbers.
Supported formats in values.yaml:
- Port range: '2000-2100'
- Address with range: '127.0.0.1:2000-2100'
- Comma-separated: '1881,1888,3000'
- Mixed: '1881,1888,2000-2100,3000'
- Object form with range: {addr: '5000-5010', tls: true}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughAdded Helm helpers to parse and expand TCP/UDP port specs (scalars, maps, ranges, comma lists) and refactored gateway templates to use normalized JSON outputs for port entries; also adjusted ConfigMap quoting and added commented examples in values.yaml. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
charts/gateway/templates/_helpers.tpl (2)
144-168: Consider consolidating TCP/UDP normalization helpers.
gateway.normalizedTcpPortsandgateway.normalizedUdpPortsshare identical logic. While Helm templating makes parameterized helpers awkward, you could potentially unify them into a singlegateway.normalizedPortshelper if the protocol distinction isn't needed within the normalization logic itself.This is a minor DRY improvement and can be deferred.
Also applies to: 175-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/gateway/templates/_helpers.tpl` around lines 144 - 168, The two helpers gateway.normalizedTcpPorts and gateway.normalizedUdpPorts duplicate identical normalization logic; consolidate them by creating a single helper named gateway.normalizedPorts that contains the shared code (reusing gateway.expandPorts for expansions) and update call sites to pass protocol if needed or ignore it if unnecessary; ensure the new helper returns the same dict "entries" JSON shape as before so consumers of normalizedTcpPorts/normalizedUdpPorts keep the same contract, then remove the duplicated helper definitions.
158-165:nodePortis silently ignored when expanding port ranges.When a user specifies a map entry with both a range and
nodePort:tcp: - addr: "5000-5010" nodePort: 31000The
nodePortvalue is silently discarded (line 161 setsnodePort: ""). This is logically correct since you cannot assign a single nodePort to multiple ports, but users may not expect this silent behavior.Consider documenting this limitation in
values.yamlnear the port range examples.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/gateway/templates/_helpers.tpl` around lines 158 - 165, The template silently drops nodePort when expanding ranges (see nodePort, addrStr and gateway.expandPorts usage where expanded.ports are appended with nodePort ""), so update the chart documentation: add a clear note in values.yaml next to the port/range examples that nodePort is ignored for comma- or dash-based ranges (e.g. "addr: '5000-5010'") and explain alternatives (provide individual port entries or omit nodePort and rely on ClusterIP/LoadBalancer), and optionally mention why (one nodePort cannot map to multiple ports) and how to map nodePorts manually if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/gateway/templates/_helpers.tpl`:
- Around line 122-129: The template currently computes $start and $end from a
dashed port range and uses $count := add1 (sub $end $start), which produces a
negative count and an empty range if the user supplies a reversed range (e.g.,
"2100-2000"); update the logic in the block that handles dashed ranges (the code
using contains, splitList, $start, $end, $count, sub, until) to detect when gt
$start $end and either swap the bounds (assign $tmp := $start; set $start =
$end; set $end = $tmp) so the range is normalized, or call the Helm fail
function with a clear error message indicating the reversed range, ensuring you
take the chosen approach consistently and preserve the subsequent use of $count
and the until loop to produce ports.
In `@charts/gateway/templates/service-gateway.yaml`:
- Around line 99-104: Add validation to reject or adjust invalid NodePort
assignments when $global.Values.gateway.stream.autoAssignNodePort is true:
ensure the value used for nodePort (currently $entry.port when
autoAssignNodePort is enabled, or $entry.nodePort otherwise) falls within the
Kubernetes NodePort range 30000–32767. Implement this by adding a Helm values
schema rule (values.schema.json) that requires nodePort or port to be an integer
within 30000–32767 whenever $global.Values.gateway.stream.autoAssignNodePort is
true, or add a pre-install/helm hook check that validates $entry.port before
rendering the service template and fails fast with a clear error referencing the
stream entry name and offending port.
---
Nitpick comments:
In `@charts/gateway/templates/_helpers.tpl`:
- Around line 144-168: The two helpers gateway.normalizedTcpPorts and
gateway.normalizedUdpPorts duplicate identical normalization logic; consolidate
them by creating a single helper named gateway.normalizedPorts that contains the
shared code (reusing gateway.expandPorts for expansions) and update call sites
to pass protocol if needed or ignore it if unnecessary; ensure the new helper
returns the same dict "entries" JSON shape as before so consumers of
normalizedTcpPorts/normalizedUdpPorts keep the same contract, then remove the
duplicated helper definitions.
- Around line 158-165: The template silently drops nodePort when expanding
ranges (see nodePort, addrStr and gateway.expandPorts usage where expanded.ports
are appended with nodePort ""), so update the chart documentation: add a clear
note in values.yaml next to the port/range examples that nodePort is ignored for
comma- or dash-based ranges (e.g. "addr: '5000-5010'") and explain alternatives
(provide individual port entries or omit nodePort and rely on
ClusterIP/LoadBalancer), and optionally mention why (one nodePort cannot map to
multiple ports) and how to map nodePorts manually if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0b6a2bbb-85f1-472e-a151-9df37dfd9af0
📒 Files selected for processing (5)
charts/gateway/templates/_helpers.tplcharts/gateway/templates/_pod.tplcharts/gateway/templates/configmap.yamlcharts/gateway/templates/service-gateway.yamlcharts/gateway/values.yaml
- Remove comma-separated support from expandPorts helper - Handle reversed port ranges (e.g., '2100-2000') by swapping bounds - Update values.yaml examples to match gateway changes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Support port range notation in
gateway.stream.tcpandgateway.stream.udpconfiguration for Kubernetes deployments.Why
Users with many TCP proxy ports (30+) need to modify configs and restart for each new port. Port range support reduces config changes. This is the Helm chart companion to api7/api7-ee-3-gateway#1272.
How
config.yamlwhere the gateway's native nginxlistendirective handles them (strings are properly quoted)."2100-2000"→"2000-2100") instead of silently producing no ports.New helper templates in
_helpers.tplgateway.expandPorts: Expands a port string (may contain ranges like"2000-2100") into a list of individual port numbersgateway.normalizedTcpPorts: Normalizes TCP array entries into expanded port objects with port/nodePort/tlsgateway.normalizedUdpPorts: Normalizes UDP array entries into expanded port objectsSupported formats in values.yaml
Backward compatibility
All existing formats (integer, string, map with addr/tls/nodePort) continue to work unchanged. Verified with
helm templateandhelm lint.Related PR
Gateway: api7/api7-ee-3-gateway#1272