Skip to content

Commit 8522721

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#2174 from shiftstack/apiserverport
⚠️ Convert APIServerPort to uint16
2 parents c02afe0 + 033532e commit 8522721

17 files changed

+134
-47
lines changed

‎api/v1alpha6/openstackcluster_conversion.go‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@ limitations under the License.
1717
package v1alpha6
1818

1919
import (
20+
"math"
2021
"reflect"
2122

2223
apiconversion "k8s.io/apimachinery/pkg/conversion"
24+
"k8s.io/utils/ptr"
2325
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
2426
ctrlconversion "sigs.k8s.io/controller-runtime/pkg/conversion"
2527

@@ -178,6 +180,11 @@ func restorev1alpha6ClusterSpec(previous *OpenStackClusterSpec, dst *OpenStackCl
178180
restorev1alpha6SubnetFilter(&previous.Subnet, &dst.Subnet)
179181

180182
restorev1alpha6NetworkFilter(&previous.Network, &dst.Network)
183+
184+
// APIServerPort is now uint16
185+
if previous.APIServerPort > math.MaxUint16 {
186+
dst.APIServerPort = previous.APIServerPort
187+
}
181188
}
182189

183190
func restorev1beta1ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *infrav1.OpenStackClusterSpec) {
@@ -241,7 +248,7 @@ func restorev1beta1ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *infr
241248

242249
optional.RestoreString(&previous.APIServerFloatingIP, &dst.APIServerFloatingIP)
243250
optional.RestoreString(&previous.APIServerFixedIP, &dst.APIServerFixedIP)
244-
optional.RestoreInt(&previous.APIServerPort, &dst.APIServerPort)
251+
optional.RestoreUInt16(&previous.APIServerPort, &dst.APIServerPort)
245252
optional.RestoreBool(&previous.DisableAPIServerFloatingIP, &dst.DisableAPIServerFloatingIP)
246253
optional.RestoreBool(&previous.ControlPlaneOmitAvailabilityZone, &dst.ControlPlaneOmitAvailabilityZone)
247254
optional.RestoreBool(&previous.DisablePortSecurity, &dst.DisablePortSecurity)
@@ -313,6 +320,10 @@ func Convert_v1alpha6_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *O
313320
out.APIServerLoadBalancer = apiServerLoadBalancer
314321
}
315322

323+
if in.APIServerPort > 0 && in.APIServerPort < math.MaxUint16 {
324+
out.APIServerPort = ptr.To(uint16(in.APIServerPort)) //nolint:gosec
325+
}
326+
316327
return nil
317328
}
318329

@@ -376,6 +387,8 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec(in *i
376387
}
377388
}
378389

390+
out.APIServerPort = int(ptr.Deref(in.APIServerPort, 0))
391+
379392
return nil
380393
}
381394

‎api/v1alpha6/zz_generated.conversion.go‎

Lines changed: 2 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/v1alpha7/openstackcluster_conversion.go‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@ limitations under the License.
1717
package v1alpha7
1818

1919
import (
20+
"math"
2021
"reflect"
2122

2223
apiconversion "k8s.io/apimachinery/pkg/conversion"
24+
"k8s.io/utils/ptr"
2325
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
2426
ctrlconversion "sigs.k8s.io/controller-runtime/pkg/conversion"
2527

@@ -168,6 +170,11 @@ func restorev1alpha7ClusterSpec(previous *OpenStackClusterSpec, dst *OpenStackCl
168170
}
169171

170172
restorev1alpha7NetworkFilter(&previous.Network, &dst.Network)
173+
174+
// APIServerPort is not uint16
175+
if previous.APIServerPort > math.MaxUint16 {
176+
dst.APIServerPort = previous.APIServerPort
177+
}
171178
}
172179

173180
func restorev1beta1ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *infrav1.OpenStackClusterSpec) {
@@ -238,7 +245,7 @@ func restorev1beta1ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *infr
238245

239246
optional.RestoreString(&previous.APIServerFloatingIP, &dst.APIServerFloatingIP)
240247
optional.RestoreString(&previous.APIServerFixedIP, &dst.APIServerFixedIP)
241-
optional.RestoreInt(&previous.APIServerPort, &dst.APIServerPort)
248+
optional.RestoreUInt16(&previous.APIServerPort, &dst.APIServerPort)
242249
optional.RestoreBool(&previous.DisableAPIServerFloatingIP, &dst.DisableAPIServerFloatingIP)
243250
optional.RestoreBool(&previous.ControlPlaneOmitAvailabilityZone, &dst.ControlPlaneOmitAvailabilityZone)
244251
optional.RestoreBool(&previous.DisablePortSecurity, &dst.DisablePortSecurity)
@@ -310,6 +317,10 @@ func Convert_v1alpha7_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *O
310317
out.APIServerLoadBalancer = apiServerLoadBalancer
311318
}
312319

320+
if in.APIServerPort > 0 && in.APIServerPort < math.MaxUint16 {
321+
out.APIServerPort = ptr.To(uint16(in.APIServerPort)) //nolint:gosec
322+
}
323+
313324
return nil
314325
}
315326

@@ -373,6 +384,8 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1alpha7_OpenStackClusterSpec(in *i
373384
}
374385
}
375386

387+
out.APIServerPort = int(ptr.Deref(in.APIServerPort, 0))
388+
376389
return nil
377390
}
378391

‎api/v1alpha7/zz_generated.conversion.go‎

Lines changed: 2 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/v1beta1/openstackcluster_types.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,9 @@ type OpenStackClusterSpec struct {
133133
APIServerFixedIP optional.String `json:"apiServerFixedIP,omitempty"`
134134

135135
// APIServerPort is the port on which the listener on the APIServer
136-
// will be created
136+
// will be created. If specified, it must be an integer between 0 and 65535.
137137
// +optional
138-
APIServerPort optional.Int `json:"apiServerPort,omitempty"`
138+
APIServerPort optional.UInt16 `json:"apiServerPort,omitempty"`
139139

140140
// ManagedSecurityGroups determines whether OpenStack security groups for the cluster
141141
// will be managed by the OpenStack provider or whether pre-existing security groups will

‎api/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml‎

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml‎

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎controllers/openstackcluster_controller.go‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23-
"math"
2423
"time"
2524

2625
"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/networks"
@@ -760,10 +759,7 @@ func reconcileProvisionedNetworkComponents(networkingService *networking.Service
760759
// cluster spec.
761760
func reconcileControlPlaneEndpoint(scope *scope.WithLogger, networkingService *networking.Service, openStackCluster *infrav1.OpenStackCluster, clusterResourceName string) error {
762761
// Calculate the port that we will use for the API server
763-
apiServerPort, err := getAPIServerPort(openStackCluster)
764-
if err != nil {
765-
return err
766-
}
762+
apiServerPort := getAPIServerPort(openStackCluster)
767763

768764
// host must be set by a matching control plane endpoint provider below
769765
var host string
@@ -830,20 +826,14 @@ func reconcileControlPlaneEndpoint(scope *scope.WithLogger, networkingService *n
830826
}
831827

832828
// getAPIServerPort returns the port to use for the API server based on the cluster spec.
833-
func getAPIServerPort(openStackCluster *infrav1.OpenStackCluster) (int32, error) {
829+
func getAPIServerPort(openStackCluster *infrav1.OpenStackCluster) int32 {
834830
switch {
835831
case openStackCluster.Spec.ControlPlaneEndpoint != nil && openStackCluster.Spec.ControlPlaneEndpoint.IsValid():
836-
return openStackCluster.Spec.ControlPlaneEndpoint.Port, nil
832+
return openStackCluster.Spec.ControlPlaneEndpoint.Port
837833
case openStackCluster.Spec.APIServerPort != nil:
838-
// XXX: This highlights that we have missing validation on
839-
// APIServerPort. We should ideally add validation and change its type
840-
// to int32.
841-
if *openStackCluster.Spec.APIServerPort > math.MaxInt32 {
842-
return 0, fmt.Errorf("value of apiServerPort is larger than %d", math.MaxInt32)
843-
}
844-
return int32(*openStackCluster.Spec.APIServerPort), nil //nolint:gosec
834+
return int32(*openStackCluster.Spec.APIServerPort)
845835
}
846-
return 6443, nil
836+
return 6443
847837
}
848838

849839
func (r *OpenStackClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {

‎controllers/openstackcluster_controller_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -490,15 +490,15 @@ func Test_getAPIServerPort(t *testing.T) {
490490
name: "with API server port",
491491
openStackCluster: &infrav1.OpenStackCluster{
492492
Spec: infrav1.OpenStackClusterSpec{
493-
APIServerPort: ptr.To(6445),
493+
APIServerPort: ptr.To(uint16(6445)),
494494
},
495495
},
496496
want: 6445,
497497
},
498498
}
499499
for _, tt := range tests {
500500
t.Run(tt.name, func(t *testing.T) {
501-
if got, _ := getAPIServerPort(tt.openStackCluster); got != tt.want {
501+
if got := getAPIServerPort(tt.openStackCluster); got != tt.want {
502502
t.Errorf("getAPIServerPort() = %v, want %v", got, tt.want)
503503
}
504504
})

0 commit comments

Comments
 (0)