Skip to content

OCPBUGS-65824: Add dynamic NodePort range validation to prevent cluster creation failures#7652

Draft
jparrill wants to merge 2 commits intoopenshift:mainfrom
jparrill:fix-OCPBUGS-65824
Draft

OCPBUGS-65824: Add dynamic NodePort range validation to prevent cluster creation failures#7652
jparrill wants to merge 2 commits intoopenshift:mainfrom
jparrill:fix-OCPBUGS-65824

Conversation

@jparrill
Copy link
Contributor

@jparrill jparrill commented Feb 5, 2026

What this PR does / why we need it:

This PR adds validation for NodePort.Port to ensure ports fall within the cluster's configured ServiceNodePortRange. This prevents late failures during cluster provisioning when the control-plane-operator rejects ports outside the acceptable range.

The problem: When creating a hosted cluster with NodePort set to values like 10000, the creation fails because the port is outside the acceptable range (typically 30000-32767), but this validation only happens late in the process after resources are already created.

The solution:

  • Adds dynamic validation that reads cluster-specific ServiceNodePortRange configuration
  • Validates NodePort.Port against the configured range (supports custom ranges, not just default)
  • Allows port 0 (dynamic assignment)
  • Provides clear error messages with the actual configured range
  • Ensures early failure with immediate feedback

Which issue(s) this PR fixes:

Fixes https://issues.redhat.com/browse/OCPBUGS-65824

Special notes for your reviewer:

  • The validation is dynamic - it reads the cluster's ServiceNodePortRange configuration instead of hardcoding 30000-32767
  • This supports customers who may have custom port ranges configured
  • No CRD changes were made - validation is runtime-only to support configurable ranges
  • One existing test was updated (port 4433 → 30443) because it was using an invalid NodePort
  • All existing tests continue to pass

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs. (Function documentation explains the validation logic)
  • This change includes unit tests. (Added comprehensive tests for range parsing and validation)

🤖 Generated with Claude Code via /jira:solve OCPBUGS-65824 origin

jparrill and others added 2 commits February 5, 2026 16:26
Add validation for NodePort.Port to ensure ports fall within the
cluster's configured ServiceNodePortRange. This prevents late failures
during cluster provisioning when the control-plane-operator rejects
ports outside the acceptable range.

The validation:
- Reads cluster-specific ServiceNodePortRange or uses default (30000-32767)
- Validates NodePort.Port against the configured range
- Allows port 0 (dynamic assignment)
- Provides clear error messages with the actual configured range

Fixes an issue where specifying invalid NodePort values (e.g., 10000)
would cause cluster creation to fail after resources were created.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
Add .work/ directory to gitignore to exclude Claude Code working
directory from version control.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 5, 2026

Walkthrough

This PR adds NodePort range validation to the HostedCluster controller by introducing internal helper functions to parse and validate node port ranges. It includes comprehensive tests for the new functionality and adds a .work/ directory ignore rule to .gitignore.

Changes

Cohort / File(s) Summary
Configuration
.gitignore
Added ignore rule for .work/ directory (Claude Code working directory).
NodePort Range Validation
hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go, hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go
Introduced internal helpers parseNodePortRange() and validateNodePortPortRange() for parsing and validating NodePort ranges with defaults (30000–32767). Extended validation flow to check NodePort values. Updated existing test expectation (4433 → 30443) and added comprehensive test cases for range parsing and validation. Tests contain duplicate definitions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@jparrill
Copy link
Contributor Author

jparrill commented Feb 5, 2026

/auto-cc

@jparrill
Copy link
Contributor Author

jparrill commented Feb 6, 2026

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 6, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@jparrill
Copy link
Contributor Author

jparrill commented Feb 6, 2026

/test unit

@jparrill
Copy link
Contributor Author

jparrill commented Feb 6, 2026

/test verify

1 similar comment
@jparrill
Copy link
Contributor Author

jparrill commented Feb 6, 2026

/test verify

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 6, 2026

@jparrill: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/verify ec1cf03 link true /test verify

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant