Improve LoadBalancerClass validation

pkg/apis/openstack/cloudprofile_test.go

@@ -0,0 +1,86 @@
+// Copyright (c) 2021 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package openstack_test
+
+import (
+	. "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack"
+	"k8s.io/utils/pointer"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("LoadBalancerClass", func() {
+	Context("#IsSemanticallyEqual", func() {
+		var (
+			loadBalancerClassA LoadBalancerClass
+			loadBalancerClassB LoadBalancerClass
+		)
+
+		BeforeEach(func() {
+			loadBalancerClassA = LoadBalancerClass{
+				Name:               "lbclass-A",
+				FloatingNetworkID:  pointer.StringPtr("floating-network-id"),
+				FloatingSubnetID:   pointer.StringPtr("floating-subnet-id"),
+				FloatingSubnetName: pointer.StringPtr("floating-subnet-name"),
+				FloatingSubnetTags: pointer.StringPtr("floating-subnet-tags"),
+				SubnetID:           pointer.StringPtr("subnet-id"),
+			}
+			loadBalancerClassB = LoadBalancerClass{
+				Name:               "lbclass-B",
+				FloatingNetworkID:  pointer.StringPtr("floating-network-id"),
+				FloatingSubnetID:   pointer.StringPtr("floating-subnet-id"),
+				FloatingSubnetName: pointer.StringPtr("floating-subnet-name"),
+				FloatingSubnetTags: pointer.StringPtr("floating-subnet-tags"),
+				SubnetID:           pointer.StringPtr("subnet-id"),
+			}
+		})
+
+		It("should return true as LoadBalancerClass are semantically equal with different names", func() {
+			Expect(loadBalancerClassA.IsSemanticallyEqual(loadBalancerClassB)).To(BeTrue())
+		})
+
+		It("should return true as LoadBalancerClass are semantically equal with different purposes", func() {
+			loadBalancerClassA.Purpose = pointer.StringPtr("purpose-a")
+			loadBalancerClassB.Purpose = pointer.StringPtr("purpose-b")
+			Expect(loadBalancerClassA.IsSemanticallyEqual(loadBalancerClassB)).To(BeTrue())
+		})
+
+		It("should return false as LoadBalancerClass are not semantically due to different floating network ids", func() {
+			loadBalancerClassB.FloatingNetworkID = pointer.StringPtr("floating-network-id-2")
+			Expect(loadBalancerClassA.IsSemanticallyEqual(loadBalancerClassB)).To(BeFalse())
+		})
+
+		It("should return false as LoadBalancerClass are not semantically due to different floating subnet ids", func() {
+			loadBalancerClassB.FloatingSubnetID = pointer.StringPtr("floating-subnet-id-2")
+			Expect(loadBalancerClassA.IsSemanticallyEqual(loadBalancerClassB)).To(BeFalse())
+		})
+
+		It("should return false as LoadBalancerClass are not semantically due to different floating subnet names", func() {
+			loadBalancerClassB.FloatingSubnetName = pointer.StringPtr("floating-subnet-name-2")
+			Expect(loadBalancerClassA.IsSemanticallyEqual(loadBalancerClassB)).To(BeFalse())
+		})
+
+		It("should return false as LoadBalancerClass are not semantically due to different floating subnet tags", func() {
+			loadBalancerClassB.FloatingSubnetTags = pointer.StringPtr("floating-subnet-tags-2")
+			Expect(loadBalancerClassA.IsSemanticallyEqual(loadBalancerClassB)).To(BeFalse())
+		})
+
+		It("should return false as LoadBalancerClass are not semantically due to different subnet ids", func() {
+			loadBalancerClassB.SubnetID = pointer.StringPtr("subnet-ids-2")
+			Expect(loadBalancerClassA.IsSemanticallyEqual(loadBalancerClassB)).To(BeFalse())
+		})
+	})
+})

pkg/apis/openstack/openstack_suite_test.go

@@ -0,0 +1,27 @@
+// Copyright (c) 2021 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package openstack_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestV1alpha1(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "API Openstack Suite")
+}

pkg/apis/openstack/types_cloudprofile.go

@@ -17,6 +17,8 @@ package openstack
 import (
 	"fmt"
 
+	"github.com/gardener/gardener-extension-provider-openstack/pkg/utils"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -106,6 +108,27 @@ type LoadBalancerClass struct {
 	SubnetID *string
 }
 
+// IsSemanticallyEqual checks if the load balancer class is semantically equal to
+// another given load balancer class. Name and Purpose fields are allowed to be different.
+func (l LoadBalancerClass) IsSemanticallyEqual(e LoadBalancerClass) bool {
+	if !utils.StringEqual(l.FloatingNetworkID, e.FloatingNetworkID) {
+		return false
+	}
+	if !utils.StringEqual(l.FloatingSubnetID, e.FloatingSubnetID) {
+		return false
+	}
+	if !utils.StringEqual(l.FloatingSubnetName, e.FloatingSubnetName) {
+		return false
+	}
+	if !utils.StringEqual(l.FloatingSubnetTags, e.FloatingSubnetTags) {
+		return false
+	}
+	if !utils.StringEqual(l.SubnetID, e.SubnetID) {
+		return false
+	}
+	return true
+}
+
 func (in LoadBalancerClass) String() string {
 	result := fmt.Sprintf("Name: %q", in.Name)
 	if in.Purpose != nil {

pkg/apis/openstack/validation/cloudprofile.go

@@ -66,10 +66,7 @@ func ValidateCloudProfileConfig(cloudProfile *api.CloudProfileConfig, fldPath *f
 	}
 
 	for i, pool := range cloudProfile.Constraints.FloatingPools {
-		lbClassPath := floatingPoolPath.Index(i).Child("loadBalancerClasses")
-		for j, class := range pool.LoadBalancerClasses {
-			allErrs = append(allErrs, ValidateLoadBalancerClasses(class, lbClassPath.Index(j))...)
-		}
+		allErrs = append(allErrs, ValidateLoadBalancerClasses(pool.LoadBalancerClasses, floatingPoolPath.Index(i).Child("loadBalancerClasses"))...)
 	}
 
 	loadBalancerProviderPath := fldPath.Child("constraints", "loadBalancerProviders")
@@ -169,12 +166,12 @@ func ValidateCloudProfileConfig(cloudProfile *api.CloudProfileConfig, fldPath *f
 	return allErrs
 }
 
-// ValidateLoadBalancerClasses validates LoadBalancerClass object.
-func ValidateLoadBalancerClasses(lbClass api.LoadBalancerClass, fldPath *field.Path) field.ErrorList {
+// validateLoadBalancerClass validates LoadBalancerClass object.
+func validateLoadBalancerClass(lbClass api.LoadBalancerClass, fldPath *field.Path) field.ErrorList {
 	var allErrs = field.ErrorList{}
 
 	if lbClass.Purpose != nil && *lbClass.Purpose != api.DefaultLoadBalancerClass && *lbClass.Purpose != api.PrivateLoadBalancerClass && *lbClass.Purpose != api.VPNLoadBalancerClass {
-		allErrs = append(allErrs, field.Invalid(fldPath, *lbClass.Purpose, fmt.Sprintf("Invalid LoadBalancerClass purpose. Valid values are %q or %q", api.DefaultLoadBalancerClass, api.PrivateLoadBalancerClass)))
+		allErrs = append(allErrs, field.Invalid(fldPath, *lbClass.Purpose, fmt.Sprintf("invalid LoadBalancerClass purpose. Valid values are %q or %q", api.DefaultLoadBalancerClass, api.PrivateLoadBalancerClass)))
 	}
 
 	if lbClass.FloatingSubnetID != nil && lbClass.FloatingSubnetName != nil && lbClass.FloatingSubnetTags != nil {
@@ -192,3 +189,48 @@ func ValidateLoadBalancerClasses(lbClass api.LoadBalancerClass, fldPath *field.P
 
 	return allErrs
 }
+
+// ValidateLoadBalancerClasses validates a given list of LoadBalancerClass objects.
+func ValidateLoadBalancerClasses(loadBalancerClasses []api.LoadBalancerClass, fldPath *field.Path) field.ErrorList {
+	var (
+		defaultClassExists bool
+		privateClassExists bool
+
+		allErrs      = field.ErrorList{}
+		lbClassNames = sets.NewString()
+	)
+
+	for i, class := range loadBalancerClasses {
+		lbClassPath := fldPath.Index(i)
+
+		// Validate first the load balancer class itself.
+		allErrs = append(allErrs, validateLoadBalancerClass(class, lbClassPath)...)
+
+		// All load balancer classes need to have an unique name. Check for duplicates.
+		if lbClassNames.Has(class.Name) {
+			allErrs = append(allErrs, field.Duplicate(lbClassPath.Child("name"), class.Name))
+		} else {
+			lbClassNames.Insert(class.Name)
+		}
+
+		// There can only be one default load balancer class. Check for multiple default classes.
+		if (class.Purpose != nil && *class.Purpose == api.DefaultLoadBalancerClass) || class.Name == api.DefaultLoadBalancerClass {
+			if defaultClassExists {
+				allErrs = append(allErrs, field.Invalid(fldPath, loadBalancerClasses, "not allowed to configure multiple default load balancer classes"))
+			} else {
+				defaultClassExists = true
+			}
+		}
+
+		// There can only be one private load balancer class. Check for multiple private classes.
+		if (class.Purpose != nil && *class.Purpose == api.PrivateLoadBalancerClass) || class.Name == api.PrivateLoadBalancerClass {
+			if privateClassExists {
+				allErrs = append(allErrs, field.Invalid(fldPath, loadBalancerClasses, "not allowed to configure multiple private load balancer classes"))
+			} else {
+				privateClassExists = true
+			}
+		}
+	}
+
+	return allErrs
+}