api/v1alpha1/proxmoxcluster_types: validate controlplane port (#241)

* api/v1alpha1/proxmoxcluster_types: validate controlplane port

* pkg/scope/cluster: remove unused clusterScope.ControlPlaneEndpoint (make coverage happy)

* api/v1alpha1/proxmoxcluster_types: add port 0 test

---------

Co-authored-by: Felix Wischke (65278) <[email protected]>

Author: 65278

api/v1alpha1/proxmoxcluster_types.go

@@ -37,7 +37,8 @@ const (
 type ProxmoxClusterSpec struct {
 	// ControlPlaneEndpoint represents the endpoint used to communicate with the control plane.
 	// +optional
-	ControlPlaneEndpoint clusterv1.APIEndpoint `json:"controlPlaneEndpoint"`
+	// +kubebuilder:validation:XValidation:rule="self.port > 0 && self.port < 65536",message="port must be within 1-65535"
+	ControlPlaneEndpoint *clusterv1.APIEndpoint `json:"controlPlaneEndpoint"`
 
 	// AllowedNodes specifies all Proxmox nodes which will be considered
 	// for operations. This implies that VMs can be cloned on different nodes from

api/v1alpha1/proxmoxcluster_types_test.go

@@ -26,6 +26,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ipamicv1 "sigs.k8s.io/cluster-api-ipam-provider-in-cluster/api/v1alpha2"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -106,6 +107,24 @@ var _ = Describe("ProxmoxCluster Test", func() {
 		Expect(client.IgnoreNotFound(err)).To(Succeed())
 	})
 
+	Context("ClusterPort", func() {
+		It("Should not allow ports higher than 65535", func() {
+			dc := defaultCluster()
+			dc.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{
+				Port: 65536,
+			}
+			Expect(k8sClient.Create(context.Background(), dc)).Should(MatchError(ContainSubstring("port must be within 1-65535")))
+		})
+
+		It("Should not allow port 0", func() {
+			dc := defaultCluster()
+			dc.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{
+				Port: 0,
+			}
+			Expect(k8sClient.Create(context.Background(), dc)).Should(MatchError(ContainSubstring("port must be within 1-65535")))
+		})
+	})
+
 	Context("IPv4Config", func() {
 		It("Should not allow empty addresses", func() {
 			dc := defaultCluster()

api/v1alpha1/zz_generated.deepcopy.go

@@ -561,6 +561,9 @@ spec:
                 - host
                 - port
                 type: object
+                x-kubernetes-validations:
+                - message: port must be within 1-65535
+                  rule: self.port > 0 && self.port < 65536
               credentialsRef:
                 description: CredentialsRef is a reference to a Secret that contains
                   the credentials to use for provisioning this cluster. If not supplied

config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclusters.yaml

@@ -610,6 +610,9 @@ spec:
                         - host
                         - port
                         type: object
+                        x-kubernetes-validations:
+                        - message: port must be within 1-65535
+                          rule: self.port > 0 && self.port < 65536
                       credentialsRef:
                         description: CredentialsRef is a reference to a Secret that
                           contains the credentials to use for provisioning this cluster.

config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxclustertemplates.yaml

@@ -228,7 +228,7 @@ func buildProxmoxCluster(name string) infrav1.ProxmoxCluster {
 			},
 		},
 		Spec: infrav1.ProxmoxClusterSpec{
-			ControlPlaneEndpoint: clusterv1.APIEndpoint{
+			ControlPlaneEndpoint: &clusterv1.APIEndpoint{
 				Host: "10.10.10.11",
 				Port: 6443,
 			},

internal/controller/proxmoxcluster_controller_test.go

@@ -127,21 +127,6 @@ func validateControlPlaneEndpoint(cluster *infrav1.ProxmoxCluster) error {
 					field.NewPath("spec", "controlplaneEndpoint"), endpoint, "provided endpoint address is not a valid IP or FQDN"),
 			})
 	}
-	// If the passed control-plane endppoint is an IPv6 address, wrap it in [], so it can properly pass ParseAddrPort validation
-	if addr.Is6() {
-		endpoint = fmt.Sprintf("[%s]", endpoint)
-	}
-
-	ipAddr, err := netip.ParseAddrPort(fmt.Sprintf("%s:%d", endpoint, ep.Port))
-	if err != nil {
-		return apierrors.NewInvalid(
-			gk,
-			name,
-			field.ErrorList{
-				field.Invalid(
-					field.NewPath("spec", "controlplaneEndpoint"), fmt.Sprintf("%s:%d", endpoint, ep.Port), "provided endpoint is not in a valid IP and port format"),
-			})
-	}
 
 	// IPv4
 	if cluster.Spec.IPv4Config != nil {
@@ -156,7 +141,7 @@ func validateControlPlaneEndpoint(cluster *infrav1.ProxmoxCluster) error {
 				})
 		}
 
-		if set.Contains(ipAddr.Addr()) {
+		if set.Contains(addr) {
 			return apierrors.NewInvalid(
 				gk,
 				name,
@@ -180,7 +165,7 @@ func validateControlPlaneEndpoint(cluster *infrav1.ProxmoxCluster) error {
 				})
 		}
 
-		if set6.Contains(ipAddr.Addr()) {
+		if set6.Contains(addr) {
 			return apierrors.NewInvalid(
 				gk,
 				name,

internal/webhook/proxmoxcluster_webhook.go

@@ -79,13 +79,6 @@ var _ = Describe("Controller Test", func() {
 			g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(Succeed())
 		})
 
-		It("should disallow invalid endpoint IP + port combination", func() {
-			cluster := invalidProxmoxCluster("test-cluster")
-			cluster.Spec.ControlPlaneEndpoint.Host = "127.0.0.1"
-			cluster.Spec.ControlPlaneEndpoint.Port = 69000
-			g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(MatchError(ContainSubstring("provided endpoint is not in a valid IP and port format")))
-		})
-
 		It("should disallow invalid IPV4 IPs", func() {
 			cluster := invalidProxmoxCluster("test-cluster")
 			cluster.Spec.IPv4Config.Addresses = []string{"invalid"}
@@ -141,7 +134,7 @@ func validProxmoxCluster(name string) infrav1.ProxmoxCluster {
 			Namespace: metav1.NamespaceDefault,
 		},
 		Spec: infrav1.ProxmoxClusterSpec{
-			ControlPlaneEndpoint: clusterv1.APIEndpoint{
+			ControlPlaneEndpoint: &clusterv1.APIEndpoint{
 				Host: "10.10.10.1",
 				Port: 6443,
 			},
@@ -159,7 +152,7 @@ func validProxmoxCluster(name string) infrav1.ProxmoxCluster {
 
 func invalidProxmoxCluster(name string) infrav1.ProxmoxCluster {
 	cl := validProxmoxCluster(name)
-	cl.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{
+	cl.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{
 		Host: "10.10.10.2",
 		Port: 6443,
 	}

internal/webhook/proxmoxcluster_webhook_test.go

@@ -179,11 +179,6 @@ func (s *ClusterScope) KubernetesClusterName() string {
 	return s.Cluster.Name
 }
 
-// ControlPlaneEndpoint returns the ControlPlaneEndpoint for the associated ProxmoxCluster.
-func (s *ClusterScope) ControlPlaneEndpoint() clusterv1.APIEndpoint {
-	return s.ProxmoxCluster.Spec.ControlPlaneEndpoint
-}
-
 // PatchObject persists the cluster configuration and status.
 func (s *ClusterScope) PatchObject() error {
 	// always update the readyCondition.