fix(helm): validate gateway listener ports by guanchzhou · Pull Request #3179 · NVIDIA/OpenShell

guanchzhou · 2026-09-04T08:40:10Z

Summary

Reject invalid gateway listener port values during Helm rendering, before Kubernetes admission or gateway startup.

Related Issue

Closes #5475

Changes

Reject booleans, fractional numbers, and other malformed port values before integer conversion
Require the primary and probe-backed health listener ports to be valid TCP ports
Allow metrics port 0 as disabled and validate enabled metrics ports
Reject collisions between primary, health, and metrics listeners
Add focused Helm unit tests for malformed values, invalid ranges, boundaries, and collisions

Testing

mise run pre-commit passes
Unit tests added/updated
E2E tests added/updated (not applicable)
helm unittest deploy/helm/openshell (133 tests)
helm unittest deploy/helm/openshell-workspace (4 tests)
Helm lint passes for defaults and all CI values variants
Direct deployment render rejects malformed listener ports

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture docs updated (not applicable)

copy-pr-bot · 2026-09-04T08:40:14Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

github-actions · 2026-09-04T08:40:24Z

All contributors have signed the DCO ✍️ ✅
_{Posted by the DCO Assistant Lite bot.}

guanchzhou · 2026-09-04T08:40:53Z

I have read the DCO document and I hereby sign the DCO.

guanchzhou · 2026-09-04T08:41:07Z

recheck

github-actions · 2026-09-19T08:29:06Z

This pull request has had no activity for 14 days and is now marked stale. It may be closed in 7 days if there is no further activity.

guanchzhou · 2026-09-19T08:35:15Z

not stale

sylvesterkaczmarek

service.metricsPort: false still slips through this validation. Helm's default treats false as empty, so it becomes 0 before the type check. Could this distinguish a missing value from an explicit boolean and add a false case?

guanchzhou · 2026-09-19T15:10:54Z

@sylvesterkaczmarek good catch. Helm's default treats false as empty, so the previous fallback turned service.metricsPort: false into 0 before the integer check.

Fixed in d8cd988. The "0" fallback now applies only when the key is absent; an explicit value is stringified as-is, so false stays false and fails the integer validation. Added a service.metricsPort: false unit case. Helm tests and lint pass on this head.

sylvesterkaczmarek

Rechecked 93d0317. metricsPort now distinguishes an absent key from an explicit value before string conversion, so false remains "false" and fails the integer check instead of being defaulted to 0. The focused Helm regression covers the exact case I raised. My concern is resolved.

github-actions · 2026-10-04T10:18:39Z

This pull request has had no activity for 14 days and is now marked stale. It may be closed in 7 days if there is no further activity.

guanchzhou · 2026-10-04T10:20:17Z

Not stale.

…

On 4. Oct 2026 at 13.19 +0300, github-actions[bot] ***@***.***>, wrote: github-actions[bot] left a comment (NVIDIA/OpenShell#3179) This pull request has had no activity for 14 days and is now marked stale. It may be closed in 7 days if there is no further activity. — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today! You are receiving this because you authored the thread.Message ID: ***@***.***>

Reject disabled required listeners, out-of-range values, and port collisions before Kubernetes accepts an unusable gateway workload. Signed-off-by: Andrey Maltsev <maltsev.andrey@gmail.com>

Validate raw listener port values before integer conversion so booleans and fractional numbers cannot bypass range and collision checks. Signed-off-by: Andrey Maltsev <maltsev.andrey@gmail.com>

Preserve explicitly configured metrics port values during validation so boolean false cannot be converted into the disabled numeric value. Signed-off-by: Andrey Maltsev <maltsev.andrey@gmail.com>

github-actions · 2026-10-24T08:29:32Z

This pull request has had no activity for 14 days and is now marked stale.

guanchzhou · 2026-10-24T17:41:24Z

Not stale.

github-actions · 2026-11-09T08:25:07Z

This pull request has had no activity for 14 days and is now marked stale.

guanchzhou · 2026-11-09T14:15:54Z

Not stale.

TaylorMutch

Reviewed commit 62866fc. Approving the localized listener-port validation. All 462 chart tests and Helm lint passed with this patch applied to the current checkout. Oversized-integer overflow hardening is intentionally non-blocking for this change. Separating metrics enablement from the numeric port is tracked independently in #5474.

TaylorMutch · 2026-11-10T23:11:24Z

/ok to test 62866fc

guanchzhou requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners September 4, 2026 08:40

github-actions Bot added the state:stale Inactive item at risk of automatic closure. label Sep 19, 2026

sylvesterkaczmarek reviewed Sep 19, 2026

View reviewed changes

guanchzhou force-pushed the helm-gateway-port-validation/am branch from d8cd988 to 93d0317 Compare September 19, 2026 15:17

sylvesterkaczmarek approved these changes Sep 19, 2026

View reviewed changes

github-actions Bot removed the state:stale Inactive item at risk of automatic closure. label Sep 20, 2026

gmenher mentioned this pull request Sep 24, 2026

feat(metrics): opt-in secure gateway metrics endpoint #3664

Closed

11 tasks

github-actions Bot added the state:stale Inactive item at risk of automatic closure. label Oct 4, 2026

github-actions Bot removed the state:stale Inactive item at risk of automatic closure. label Oct 5, 2026

guanchzhou added 3 commits October 9, 2026 12:10

fix(helm): validate gateway listener ports

a123695

Reject disabled required listeners, out-of-range values, and port collisions before Kubernetes accepts an unusable gateway workload. Signed-off-by: Andrey Maltsev <maltsev.andrey@gmail.com>

fix(helm): reject malformed gateway port values

96bff6f

Validate raw listener port values before integer conversion so booleans and fractional numbers cannot bypass range and collision checks. Signed-off-by: Andrey Maltsev <maltsev.andrey@gmail.com>

fix(helm): reject false metrics port

62866fc

Preserve explicitly configured metrics port values during validation so boolean false cannot be converted into the disabled numeric value. Signed-off-by: Andrey Maltsev <maltsev.andrey@gmail.com>

guanchzhou force-pushed the helm-gateway-port-validation/am branch from 93d0317 to 62866fc Compare October 9, 2026 09:11

github-actions Bot added the state:stale Inactive item at risk of automatic closure. label Oct 24, 2026

github-actions Bot removed the state:stale Inactive item at risk of automatic closure. label Oct 25, 2026

github-actions Bot added the state:stale Inactive item at risk of automatic closure. label Nov 9, 2026

github-actions Bot removed the state:stale Inactive item at risk of automatic closure. label Nov 10, 2026

This was referenced Nov 10, 2026

feat(helm): separate metrics enablement from the listener port #5474

Closed

bug(helm): reject invalid gateway listener ports during rendering #5475

Closed

TaylorMutch approved these changes Nov 10, 2026

View reviewed changes

TaylorMutch added this pull request to the merge queue Nov 10, 2026

TaylorMutch mentioned this pull request Nov 10, 2026

feat(helm): separate metrics enablement from listener port #5478

Merged

8 of 9 tasks

Merged via the queue into NVIDIA:main with commit 4ad3ce9 Nov 10, 2026
69 checks passed

Conversation

guanchzhou commented Sep 4, 2026 • edited by TaylorMutch Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guanchzhou commented Sep 4, 2026

Uh oh!

guanchzhou commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 19, 2026

Uh oh!

guanchzhou commented Sep 19, 2026

Uh oh!

sylvesterkaczmarek left a comment

Choose a reason for hiding this comment

Uh oh!

guanchzhou commented Sep 19, 2026

Uh oh!

sylvesterkaczmarek left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 4, 2026

Uh oh!

guanchzhou commented Oct 4, 2026 via email

Uh oh!

github-actions Bot commented Oct 24, 2026

Uh oh!

guanchzhou commented Oct 24, 2026

Uh oh!

github-actions Bot commented Nov 9, 2026

Uh oh!

guanchzhou commented Nov 9, 2026

Uh oh!

TaylorMutch left a comment

Choose a reason for hiding this comment

Uh oh!

TaylorMutch commented Nov 10, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

guanchzhou commented Sep 4, 2026 •

edited by TaylorMutch

Loading

github-actions Bot commented Sep 4, 2026 •

edited

Loading