diff --git a/api/go.mod b/api/go.mod index f6f24a38..d0ed4214 100644 --- a/api/go.mod +++ b/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 diff --git a/api/go.sum b/api/go.sum index ad124bd1..4c232cf2 100644 --- a/api/go.sum +++ b/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= diff --git a/api/v1beta1/neutronapi_webhook.go b/api/v1beta1/neutronapi_webhook.go index acb26dae..4068ea0a 100644 --- a/api/v1beta1/neutronapi_webhook.go +++ b/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) diff --git a/go.mod b/go.mod index 45e1c9fa..731687b3 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 48dd2885..6dd2423e 100644 --- a/go.sum +++ b/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= diff --git a/test/functional/neutronapi_controller_test.go b/test/functional/neutronapi_controller_test.go index f90c6516..a27ed974 100644 --- a/test/functional/neutronapi_controller_test.go +++ b/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"), + ) + }) + }) +})