Added webhook for validating port against 6443 and kubebuilder tags t…

…o avoid duplicates. (#1746)
kubernetes-sigs · Jun 4, 2024 · ff78637 · ff78637
1 parent cba8d9e
commit ff78637
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 8 deletions.
diff --git a/api/v1beta2/ibmpowervscluster_webhook.go b/api/v1beta2/ibmpowervscluster_webhook.go
@@ -17,6 +17,7 @@ limitations under the License.
 package v1beta2
 
 import (
+	"fmt"
 	"strconv"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -78,7 +79,7 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSCluster() (admission.Warnings, err
 	}
 
 	if err := r.validateIBMPowerVSClusterCreateInfraPrereq(); err != nil {
-		allErrs = append(allErrs, err)
+		allErrs = append(allErrs, err...)
 	}
 
 	if len(allErrs) == 0 {
@@ -97,7 +98,31 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSClusterNetwork() *field.Error {
 	return nil
 }
 
-func (r *IBMPowerVSCluster) validateIBMPowerVSClusterCreateInfraPrereq() *field.Error {
+func (r *IBMPowerVSCluster) validateIBMPowerVSClusterLoadBalancerNames() (allErrs field.ErrorList) {
+	found := make(map[string]bool)
+	for i, loadbalancer := range r.Spec.LoadBalancers {
+		if found[loadbalancer.Name] {
+			allErrs = append(allErrs, field.Duplicate(field.NewPath("spec", fmt.Sprintf("loadbalancers[%d]", i)), map[string]interface{}{"Name": loadbalancer.Name}))
+		}
+		found[loadbalancer.Name] = true
+	}
+
+	return allErrs
+}
+
+func (r *IBMPowerVSCluster) validateIBMPowerVSClusterVPCSubnetNames() (allErrs field.ErrorList) {
+	found := make(map[string]bool)
+	for i, subnet := range r.Spec.VPCSubnets {
+		if found[*subnet.Name] {
+			allErrs = append(allErrs, field.Duplicate(field.NewPath("spec", fmt.Sprintf("vpcSubnets[%d]", i)), map[string]interface{}{"Name": *subnet.Name}))
+		}
+		found[*subnet.Name] = true
+	}
+
+	return allErrs
+}
+
+func (r *IBMPowerVSCluster) validateIBMPowerVSClusterCreateInfraPrereq() (allErrs field.ErrorList) {
 	annotations := r.GetAnnotations()
 	if len(annotations) == 0 {
 		return nil
@@ -110,28 +135,35 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSClusterCreateInfraPrereq() *field.
 
 	createInfra, err := strconv.ParseBool(value)
 	if err != nil {
-		return field.Invalid(field.NewPath("annotations"), r.Annotations, "value of powervs.cluster.x-k8s.io/create-infra should be boolean")
+		allErrs = append(allErrs, field.Invalid(field.NewPath("annotations"), r.Annotations, "value of powervs.cluster.x-k8s.io/create-infra should be boolean"))
 	}
 
 	if !createInfra {
 		return nil
 	}
 
 	if r.Spec.Zone == nil {
-		return field.Invalid(field.NewPath("spec.zone"), r.Spec.Zone, "value of zone is empty")
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec.zone"), r.Spec.Zone, "value of zone is empty"))
 	}
 
 	if r.Spec.VPC == nil {
-		return field.Invalid(field.NewPath("spec.vpc"), r.Spec.VPC, "value of VPC is empty")
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec.vpc"), r.Spec.VPC, "value of VPC is empty"))
 	}
 
 	if r.Spec.VPC.Region == nil {
-		return field.Invalid(field.NewPath("spec.vpc.region"), r.Spec.VPC.Region, "value of VPC region is empty")
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec.vpc.region"), r.Spec.VPC.Region, "value of VPC region is empty"))
 	}
 
 	if r.Spec.ResourceGroup == nil {
-		return field.Invalid(field.NewPath("spec.resourceGroup"), r.Spec.ResourceGroup, "value of resource group is empty")
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec.resourceGroup"), r.Spec.ResourceGroup, "value of resource group is empty"))
+	}
+	if err := r.validateIBMPowerVSClusterVPCSubnetNames(); err != nil {
+		allErrs = append(allErrs, err...)
 	}
 
-	return nil
+	if err := r.validateIBMPowerVSClusterLoadBalancerNames(); err != nil {
+		allErrs = append(allErrs, err...)
+	}
+
+	return allErrs
 }
diff --git a/api/v1beta2/ibmvpccluster_types.go b/api/v1beta2/ibmvpccluster_types.go
@@ -81,6 +81,7 @@ type VPCLoadBalancerSpec struct {
 	// +listType=map
 	// +listMapKey=port
 	// +optional
+	// ++kubebuilder:validation:UniqueItems=true
 	AdditionalListeners []AdditionalListenerSpec `json:"additionalListeners,omitempty"`
 }
 

diff --git a/cloud/scope/powervs_cluster.go b/cloud/scope/powervs_cluster.go
@@ -1858,6 +1858,7 @@ func (s *PowerVSClusterScope) ReconcileLoadBalancers() (bool, error) {
 			s.SetLoadBalancerStatus(*loadBalancer.Name, loadBalancerStatus)
 			continue
 		}
+
 		// check VPC load balancer exist in cloud
 		loadBalancerStatus, err := s.checkLoadBalancer(loadBalancer)
 		if err != nil {
@@ -1867,6 +1868,13 @@ func (s *PowerVSClusterScope) ReconcileLoadBalancers() (bool, error) {
 			s.SetLoadBalancerStatus(loadBalancer.Name, *loadBalancerStatus)
 			continue
 		}
+
+		// check loadbalancer port against apiserver port.
+		err = s.checkLoadBalancerPort(loadBalancer)
+		if err != nil {
+			return false, err
+		}
+
 		// create loadBalancer
 		s.V(3).Info("Creating VPC load balancer")
 		loadBalancerStatus, err = s.createLoadBalancer(loadBalancer)
@@ -1898,6 +1906,15 @@ func (s *PowerVSClusterScope) checkLoadBalancerStatus(lb vpcv1.LoadBalancer) boo
 	return false
 }
 
+func (s *PowerVSClusterScope) checkLoadBalancerPort(lb infrav1beta2.VPCLoadBalancerSpec) error {
+	for _, listerner := range lb.AdditionalListeners {
+		if listerner.Port == int64(s.APIServerPort()) {
+			return fmt.Errorf("port %d for the %s load balancer cannot be used as an additional listener port, as it is already assigned to the API server", listerner.Port, lb.Name)
+		}
+	}
+	return nil
+}
+
 // checkLoadBalancer checks loadBalancer in cloud.
 func (s *PowerVSClusterScope) checkLoadBalancer(lb infrav1beta2.VPCLoadBalancerSpec) (*infrav1beta2.VPCLoadBalancerStatus, error) {
 	loadBalancer, err := s.IBMVPCClient.GetLoadBalancerByName(lb.Name)