From 969af150cdc407a6cd50b0d16a16cd07b04219dd Mon Sep 17 00:00:00 2001 From: Juan Manuel Parrilla Madrid Date: Thu, 5 Feb 2026 16:26:46 +0100 Subject: [PATCH 1/2] feat(operator): add dynamic NodePort range validation 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 Signed-off-by: Juan Manuel Parrilla Madrid --- .../hostedcluster/hostedcluster_controller.go | 78 +++++ .../hostedcluster_controller_test.go | 287 +++++++++++++++++- 2 files changed, 364 insertions(+), 1 deletion(-) diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 43b1a3377f7..f045094a0aa 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -3921,6 +3921,7 @@ func (r *HostedClusterReconciler) validateNetworks(hc *hyperv1.HostedCluster) er errs = append(errs, validateSliceNetworkCIDRs(hc)...) errs = append(errs, checkAdvertiseAddressOverlapping(hc)...) errs = append(errs, validateNodePortVsServiceNetwork(hc)...) + errs = append(errs, validateNodePortPortRange(hc)...) return errs.ToAggregate() } @@ -4110,6 +4111,83 @@ func validateNodePortVsServiceNetwork(hc *hyperv1.HostedCluster) field.ErrorList return errs } +// parseNodePortRange parses a ServiceNodePortRange string like "30000-32767" into min/max values. +// This range defines which ports on worker nodes are available for NodePort services. +// The range is configurable per cluster via spec.configuration.network.serviceNodePortRange, +// with a default of 30000-32767 (following Kubernetes standard). +func parseNodePortRange(rangeStr string) (min, max int32, err error) { + if rangeStr == "" { + return 30000, 32767, nil // Default range + } + + parts := strings.Split(rangeStr, "-") + if len(parts) != 2 { + return 0, 0, fmt.Errorf("invalid range format: %s", rangeStr) + } + + minVal, err := strconv.ParseInt(parts[0], 10, 32) + if err != nil { + return 0, 0, fmt.Errorf("invalid minimum port: %s", parts[0]) + } + + maxVal, err := strconv.ParseInt(parts[1], 10, 32) + if err != nil { + return 0, 0, fmt.Errorf("invalid maximum port: %s", parts[1]) + } + + return int32(minVal), int32(maxVal), nil +} + +// validateNodePortPortRange validates that NodePort.Port is within the configured ServiceNodePortRange. +// +// NodePort services expose applications on worker nodes using a specific port. The available port range +// is controlled by the kube-apiserver's --service-node-port-range flag, which prevents conflicts with +// system services and other applications running on nodes. +// +// This validation ensures: +// 1. Specified ports fall within the cluster's configured range (preventing kube-apiserver rejection) +// 2. Port 0 (dynamic assignment) is always allowed (kube-apiserver will auto-assign from valid range) +// 3. Early feedback to users instead of late failure during cluster provisioning +// +// The range is configurable via spec.configuration.network.serviceNodePortRange, defaulting to +// the Kubernetes standard of 30000-32767. +func validateNodePortPortRange(hc *hyperv1.HostedCluster) field.ErrorList { + var errs field.ErrorList + + // Get the configured NodePort range for this cluster + nodePortRange := "" + if hc.Spec.Configuration != nil && hc.Spec.Configuration.Network != nil { + nodePortRange = hc.Spec.Configuration.Network.ServiceNodePortRange + } + if nodePortRange == "" { + nodePortRange = config.DefaultServiceNodePortRange + } + + // Parse the range + minPort, maxPort, err := parseNodePortRange(nodePortRange) + if err != nil { + errs = append(errs, field.Invalid( + field.NewPath("spec.configuration.network.serviceNodePortRange"), + nodePortRange, + fmt.Sprintf("invalid ServiceNodePortRange format: %v", err))) + return errs + } + + // Validate NodePort.Port against the configured range + for idx, svc := range hc.Spec.Services { + if svc.Service == hyperv1.APIServer && svc.Type == hyperv1.NodePort && svc.NodePort != nil { + port := svc.NodePort.Port + if port > 0 && (port < minPort || port > maxPort) { + errs = append(errs, field.Invalid( + field.NewPath(fmt.Sprintf("spec.services[%d].nodePort.port", idx)), + port, + fmt.Sprintf("port must be within the configured range %d-%d or 0 for dynamic assignment", minPort, maxPort))) + } + } + } + return errs +} + func validateSliceNetworkCIDRs(hc *hyperv1.HostedCluster) field.ErrorList { var cidrEntries []cidrEntry diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go index ba02f64a84f..b7098ecdb58 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go @@ -2123,7 +2123,7 @@ func TestValidateConfigAndClusterCapabilities(t *testing.T) { Type: hyperv1.NodePort, NodePort: &hyperv1.NodePortPublishingStrategy{ Address: "172.16.3.3", - Port: 4433, + Port: 30443, }, }, }, @@ -5065,6 +5065,291 @@ func TestValidateNodePortVsServiceNetwork(t *testing.T) { } } +func TestParseNodePortRange(t *testing.T) { + testCases := []struct { + name string + rangeStr string + expectedMin int32 + expectedMax int32 + expectError bool + }{ + { + name: "empty range uses default", + rangeStr: "", + expectedMin: 30000, + expectedMax: 32767, + expectError: false, + }, + { + name: "valid default range", + rangeStr: "30000-32767", + expectedMin: 30000, + expectedMax: 32767, + expectError: false, + }, + { + name: "valid custom range", + rangeStr: "25000-35000", + expectedMin: 25000, + expectedMax: 35000, + expectError: false, + }, + { + name: "valid small range", + rangeStr: "31000-31010", + expectedMin: 31000, + expectedMax: 31010, + expectError: false, + }, + { + name: "invalid format - no dash", + rangeStr: "30000", + expectError: true, + }, + { + name: "invalid format - multiple dashes", + rangeStr: "30000-31000-32000", + expectError: true, + }, + { + name: "invalid minimum port", + rangeStr: "abc-32767", + expectError: true, + }, + { + name: "invalid maximum port", + rangeStr: "30000-xyz", + expectError: true, + }, + { + name: "negative port", + rangeStr: "-1-32767", + expectError: true, + }, + { + name: "port too large", + rangeStr: "30000-99999", + expectedMin: 30000, + expectedMax: 99999, + expectError: false, // parseNodePortRange doesn't validate port limits + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + min, max, err := parseNodePortRange(tc.rangeStr) + if tc.expectError { + if err == nil { + t.Errorf("expected error but got none") + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if min != tc.expectedMin { + t.Errorf("expected min %d, got %d", tc.expectedMin, min) + } + if max != tc.expectedMax { + t.Errorf("expected max %d, got %d", tc.expectedMax, max) + } + } + }) + } +} + +func TestValidateNodePortPortRange(t *testing.T) { + testCases := []struct { + name string + hostedCluster *hyperv1.HostedCluster + expectedErrorList field.ErrorList + }{ + { + name: "valid port in default range", + hostedCluster: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.NodePort, + NodePort: &hyperv1.NodePortPublishingStrategy{ + Address: "1.1.1.1", + Port: 31000, + }, + }, + }, + }, + }, + }, + }, + { + name: "port 0 for dynamic assignment - always valid", + hostedCluster: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.NodePort, + NodePort: &hyperv1.NodePortPublishingStrategy{ + Address: "1.1.1.1", + Port: 0, + }, + }, + }, + }, + }, + }, + }, + { + name: "port below default range", + hostedCluster: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.NodePort, + NodePort: &hyperv1.NodePortPublishingStrategy{ + Address: "1.1.1.1", + Port: 10000, + }, + }, + }, + }, + }, + }, + expectedErrorList: field.ErrorList{ + field.Invalid(field.NewPath("spec.services[0].nodePort.port"), int32(10000), "port must be within the configured range 30000-32767 or 0 for dynamic assignment"), + }, + }, + { + name: "port above default range", + hostedCluster: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.NodePort, + NodePort: &hyperv1.NodePortPublishingStrategy{ + Address: "1.1.1.1", + Port: 65000, + }, + }, + }, + }, + }, + }, + expectedErrorList: field.ErrorList{ + field.Invalid(field.NewPath("spec.services[0].nodePort.port"), int32(65000), "port must be within the configured range 30000-32767 or 0 for dynamic assignment"), + }, + }, + { + name: "valid port in custom range", + hostedCluster: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Configuration: &hyperv1.ClusterConfiguration{ + Network: &configv1.NetworkSpec{ + ServiceNodePortRange: "25000-35000", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.NodePort, + NodePort: &hyperv1.NodePortPublishingStrategy{ + Address: "1.1.1.1", + Port: 28000, + }, + }, + }, + }, + }, + }, + }, + { + name: "port outside custom range", + hostedCluster: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Configuration: &hyperv1.ClusterConfiguration{ + Network: &configv1.NetworkSpec{ + ServiceNodePortRange: "25000-35000", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.NodePort, + NodePort: &hyperv1.NodePortPublishingStrategy{ + Address: "1.1.1.1", + Port: 40000, + }, + }, + }, + }, + }, + }, + expectedErrorList: field.ErrorList{ + field.Invalid(field.NewPath("spec.services[0].nodePort.port"), int32(40000), "port must be within the configured range 25000-35000 or 0 for dynamic assignment"), + }, + }, + { + name: "invalid range format", + hostedCluster: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Configuration: &hyperv1.ClusterConfiguration{ + Network: &configv1.NetworkSpec{ + ServiceNodePortRange: "invalid-range", + }, + }, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.NodePort, + NodePort: &hyperv1.NodePortPublishingStrategy{ + Address: "1.1.1.1", + Port: 31000, + }, + }, + }, + }, + }, + }, + expectedErrorList: field.ErrorList{ + field.Invalid(field.NewPath("spec.configuration.network.serviceNodePortRange"), "invalid-range", "invalid ServiceNodePortRange format: invalid minimum port: invalid"), + }, + }, + { + name: "no nodeport service - no validation", + hostedCluster: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + }, + }, + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := validateNodePortPortRange(tc.hostedCluster) + if diff := cmp.Diff(actual, tc.expectedErrorList, equateErrorMessage); diff != "" { + t.Errorf("actual validation result differs from expected: %s", diff) + } + }) + } +} + func TestServiceAccountSigningKeyBytes(t *testing.T) { g := NewWithT(t) From ec1cf0355f675017ad0a375d6bc5f85eb2a9bc7f Mon Sep 17 00:00:00 2001 From: Juan Manuel Parrilla Madrid Date: Thu, 5 Feb 2026 16:29:59 +0100 Subject: [PATCH 2/2] chore: add .work/ directory to gitignore Add .work/ directory to gitignore to exclude Claude Code working directory from version control. Co-Authored-By: Claude Sonnet 4 Signed-off-by: Juan Manuel Parrilla Madrid --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 318ddc7c1d5..b2890e10bf0 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,9 @@ tools/bin # Mocked interface generate through mockgen *_mock.go +# Claude Code working directory +.work/ + # dev dev/