Check for valid service override endpoint type on create and update

Depends-On: openstack-k8s-operators/lib-common#505

api/go.mod

@@ -3,7 +3,7 @@ module github.com/openstack-k8s-operators/neutron-operator/api
 go 1.20
 
 require (
-	github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240429052447-09a614506ca6
+	github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240522141801-d6e03083e82a
 	github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240429052447-09a614506ca6
 	k8s.io/api v0.28.9
 	k8s.io/apimachinery v0.28.9

api/go.sum

@@ -65,8 +65,8 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
 github.com/onsi/ginkgo/v2 v2.17.2 h1:7eMhcy3GimbsA3hEnVKdw/PQM9XN9krpKVXsZdph0/g=
 github.com/onsi/gomega v1.33.0 h1:snPCflnZrpMsy94p4lXVEkHo12lmPnc3vY5XBbreexE=
-github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240429052447-09a614506ca6 h1:WLsG3Ko+phW5xZJjncypLWGASoLqKrt05qN9Zxsad6g=
-github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240429052447-09a614506ca6/go.mod h1:lYhFzul37AR/6gAhTAA1KKWbOlzB3F/7014lejn883c=
+github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240522141801-d6e03083e82a h1:kcASVA9sZg9DtggyJlN6JZE6pIenJgXivFK6ry7WUVM=
+github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240522141801-d6e03083e82a/go.mod h1:lYhFzul37AR/6gAhTAA1KKWbOlzB3F/7014lejn883c=
 github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240429052447-09a614506ca6 h1:NNSOEpTZCa9RL5sZiF4ZOlB+agBrL7q7FB9pC58d4S8=
 github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240429052447-09a614506ca6/go.mod h1:C/qUWW4lW3687riZxYd+YRCtOyHZKURu3Imv6S9OP7U=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=

api/v1beta1/neutronapi_webhook.go

@@ -23,7 +23,12 @@ limitations under the License.
 package v1beta1
 
 import (
+	"fmt"
+
+	"github.com/openstack-k8s-operators/lib-common/modules/common/service"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -88,18 +93,73 @@ var _ webhook.Validator = &NeutronAPI{}
 func (r *NeutronAPI) ValidateCreate() (admission.Warnings, error) {
 	neutronapilog.Info("validate create", "name", r.Name)
 
-	// TODO(user): fill in your validation logic upon object creation.
+	allErrs := field.ErrorList{}
+	basePath := field.NewPath("spec")
+
+	if err := r.Spec.ValidateCreate(basePath); err != nil {
+		allErrs = append(allErrs, err...)
+	}
+
+	if len(allErrs) != 0 {
+		return nil, apierrors.NewInvalid(GroupVersion.WithKind("NeutronAPI").GroupKind(), r.Name, allErrs)
+	}
+
 	return nil, nil
 }
 
+// ValidateCreate - Exported function wrapping non-exported validate functions,
+// this function can be called externally to validate an NeutronAPI spec.
+func (r *NeutronAPISpec) ValidateCreate(basePath *field.Path) field.ErrorList {
+	return r.NeutronAPISpecCore.ValidateCreate(basePath)
+}
+
+func (r *NeutronAPISpecCore) ValidateCreate(basePath *field.Path) field.ErrorList {
+	var allErrs field.ErrorList
+
+	// validate the service override key is valid
+	allErrs = append(allErrs, service.ValidateRoutedOverrides(basePath.Child("override").Child("service"), r.Override.Service)...)
+
+	return allErrs
+}
+
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
 func (r *NeutronAPI) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
 	neutronapilog.Info("validate update", "name", r.Name)
 
-	// TODO(user): fill in your validation logic upon object update.
+	oldNeutronAPI, ok := old.(*NeutronAPI)
+	if !ok || oldNeutronAPI == nil {
+		return nil, apierrors.NewInternalError(fmt.Errorf("unable to convert existing object"))
+	}
+
+	allErrs := field.ErrorList{}
+	basePath := field.NewPath("spec")
+
+	if err := r.Spec.ValidateUpdate(oldNeutronAPI.Spec, basePath); err != nil {
+		allErrs = append(allErrs, err...)
+	}
+
+	if len(allErrs) != 0 {
+		return nil, apierrors.NewInvalid(GroupVersion.WithKind("NeutronAPI").GroupKind(), r.Name, allErrs)
+	}
+
 	return nil, nil
 }
 
+// ValidateUpdate - Exported function wrapping non-exported validate functions,
+// this function can be called externally to validate an neutron spec.
+func (spec *NeutronAPISpec) ValidateUpdate(old NeutronAPISpec, basePath *field.Path) field.ErrorList {
+	return spec.NeutronAPISpecCore.ValidateUpdate(old.NeutronAPISpecCore, basePath)
+}
+
+func (spec *NeutronAPISpecCore) ValidateUpdate(old NeutronAPISpecCore, basePath *field.Path) field.ErrorList {
+	var allErrs field.ErrorList
+
+	// validate the service override key is valid
+	allErrs = append(allErrs, service.ValidateRoutedOverrides(basePath.Child("override").Child("service"), spec.Override.Service)...)
+
+	return allErrs
+}
+
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type
 func (r *NeutronAPI) ValidateDelete() (admission.Warnings, error) {
 	neutronapilog.Info("validate delete", "name", r.Name)

go.mod

@@ -10,7 +10,7 @@ require (
 	github.com/onsi/gomega v1.33.0
 	github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240429104248-25176c735750
 	github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240429164853-7e1e3b111ee9
-	github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240429052447-09a614506ca6
+	github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240522141801-d6e03083e82a
 	github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240429052447-09a614506ca6
 	github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240429052447-09a614506ca6
 	github.com/openstack-k8s-operators/mariadb-operator/api v0.3.1-0.20240429121622-952f44520872

go.sum

@@ -78,8 +78,8 @@ github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240429104248-2
 github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240429104248-25176c735750/go.mod h1:QN2DJpfEc+mbvvfhoCuJ/UhQzvw12Mf+8nS0QX1HGIg=
 github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240429164853-7e1e3b111ee9 h1:aS7xUxC/uOXfw0T4ARTu0G1qb6eJ0WnB2JRv9donPOE=
 github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240429164853-7e1e3b111ee9/go.mod h1:Y/ge/l24phVaJb9S8mYRjtnDkohFkX/KEOUXLArcyvQ=
-github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240429052447-09a614506ca6 h1:WLsG3Ko+phW5xZJjncypLWGASoLqKrt05qN9Zxsad6g=
-github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240429052447-09a614506ca6/go.mod h1:lYhFzul37AR/6gAhTAA1KKWbOlzB3F/7014lejn883c=
+github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240522141801-d6e03083e82a h1:kcASVA9sZg9DtggyJlN6JZE6pIenJgXivFK6ry7WUVM=
+github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240522141801-d6e03083e82a/go.mod h1:lYhFzul37AR/6gAhTAA1KKWbOlzB3F/7014lejn883c=
 github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240429052447-09a614506ca6 h1:/mhzQQ9FF70z00zZD7dpgOoNXvEu9q68oob3oAiJW08=
 github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240429052447-09a614506ca6/go.mod h1:mrRNYeg8jb1zgGsufpN1/IB3sdbaST8btTBLwQ+taaA=
 github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240429052447-09a614506ca6 h1:NNSOEpTZCa9RL5sZiF4ZOlB+agBrL7q7FB9pC58d4S8=

test/functional/neutronapi_controller_test.go

@@ -19,12 +19,15 @@ package functional_test
 import (
 	"encoding/json"
 	"fmt"
+	"os"
 	"strings"
 
 	. "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports
 	. "github.com/onsi/gomega"    //revive:disable:dot-imports
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	//revive:disable-next-line:dot-imports
+	"github.com/openstack-k8s-operators/lib-common/modules/common/service"
 	. "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"
 
 	"github.com/google/uuid"
@@ -33,6 +36,7 @@ import (
 	mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
 
@@ -1386,3 +1390,126 @@ var _ = Describe("NeutronAPI controller", func() {
 	})
 
 })
+
+var _ = Describe("NeutronAPI Webhook", func() {
+
+	var apiTransportURLName types.NamespacedName
+	var neutronAPIName types.NamespacedName
+	var memcachedSpec memcachedv1.MemcachedSpec
+	var memcachedName types.NamespacedName
+	var name string
+
+	BeforeEach(func() {
+		name = fmt.Sprintf("neutron-%s", uuid.New().String())
+		apiTransportURLName = types.NamespacedName{
+			Namespace: namespace,
+			Name:      name + "-neutron-transport",
+		}
+
+		neutronAPIName = types.NamespacedName{
+			Namespace: namespace,
+			Name:      name,
+		}
+		memcachedSpec = memcachedv1.MemcachedSpec{
+			MemcachedSpecCore: memcachedv1.MemcachedSpecCore{
+				Replicas: ptr.To(int32(3)),
+			},
+		}
+		memcachedName = types.NamespacedName{
+			Name:      "memcached",
+			Namespace: namespace,
+		}
+
+		err := os.Setenv("OPERATOR_TEMPLATES", "../../templates")
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("rejects with wrong NeutronAPI service override endpoint type", func() {
+		spec := GetDefaultNeutronAPISpec()
+		spec["override"] = map[string]interface{}{
+			"service": map[string]interface{}{
+				"internal": map[string]interface{}{},
+				"wrooong":  map[string]interface{}{},
+			},
+		}
+
+		raw := map[string]interface{}{
+			"apiVersion": "neutron.openstack.org/v1beta1",
+			"kind":       "NeutronAPI",
+			"metadata": map[string]interface{}{
+				"name":      neutronAPIName.Name,
+				"namespace": neutronAPIName.Namespace,
+			},
+			"spec": spec,
+		}
+
+		unstructuredObj := &unstructured.Unstructured{Object: raw}
+		_, err := controllerutil.CreateOrPatch(
+			th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil })
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(
+			ContainSubstring(
+				"invalid: spec.override.service[wrooong]: " +
+					"Invalid value: \"wrooong\": invalid endpoint type: wrooong"),
+		)
+	})
+
+	When("A NeutronAPI instance is updated with wrong service override endpoint", func() {
+		BeforeEach(func() {
+			spec := GetDefaultNeutronAPISpec()
+			DeferCleanup(th.DeleteInstance, CreateNeutronAPI(neutronAPIName.Namespace, neutronAPIName.Name, spec))
+			DeferCleanup(k8sClient.Delete, ctx, CreateNeutronAPISecret(namespace, SecretName))
+			DeferCleanup(
+				mariadb.DeleteDBService,
+				mariadb.CreateDBService(
+					namespace,
+					GetNeutronAPI(neutronAPIName).Spec.DatabaseInstance,
+					corev1.ServiceSpec{
+						Ports: []corev1.ServicePort{{Port: 3306}},
+					},
+				),
+			)
+			SimulateTransportURLReady(apiTransportURLName)
+			DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(namespace, "memcached", memcachedSpec))
+			infra.SimulateMemcachedReady(memcachedName)
+			DeferCleanup(DeleteOVNDBClusters, CreateOVNDBClusters(namespace))
+			DeferCleanup(keystone.DeleteKeystoneAPI, keystone.CreateKeystoneAPI(namespace))
+			mariadb.SimulateMariaDBAccountCompleted(types.NamespacedName{Namespace: namespace, Name: GetNeutronAPI(neutronAPIName).Spec.DatabaseAccount})
+			mariadb.SimulateMariaDBDatabaseCompleted(types.NamespacedName{Namespace: namespace, Name: neutronapi.DatabaseCRName})
+			th.SimulateJobSuccess(types.NamespacedName{Namespace: namespace, Name: neutronAPIName.Name + "-db-sync"})
+			keystone.SimulateKeystoneServiceReady(types.NamespacedName{Namespace: namespace, Name: "neutron"})
+			keystone.SimulateKeystoneEndpointReady(types.NamespacedName{Namespace: namespace, Name: "neutron"})
+			deplName := types.NamespacedName{
+				Namespace: namespace,
+				Name:      "neutron",
+			}
+			th.SimulateDeploymentReadyWithPods(
+				deplName,
+				map[string][]string{namespace + "/internalapi": {}},
+			)
+
+			th.ExpectCondition(
+				neutronAPIName,
+				ConditionGetterFunc(NeutronAPIConditionGetter),
+				condition.ReadyCondition,
+				corev1.ConditionTrue,
+			)
+		})
+
+		It("rejects update with wrong service override endpoint type", func() {
+			NeutronAPI := GetNeutronAPI(neutronAPIName)
+			Expect(NeutronAPI).NotTo(BeNil())
+			if NeutronAPI.Spec.Override.Service == nil {
+				NeutronAPI.Spec.Override.Service = map[service.Endpoint]service.RoutedOverrideSpec{}
+			}
+			NeutronAPI.Spec.Override.Service["wrooong"] = service.RoutedOverrideSpec{}
+			err := k8sClient.Update(ctx, NeutronAPI)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(
+				ContainSubstring(
+					"invalid: spec.override.service[wrooong]: " +
+						"Invalid value: \"wrooong\": invalid endpoint type: wrooong"),
+			)
+		})
+	})
+})