From e2e5f4fdd6534c71eed4f547d470a70ec0fc1b0d Mon Sep 17 00:00:00 2001 From: Martin Schuppert Date: Wed, 8 May 2024 14:52:49 +0200 Subject: [PATCH] Add webhook to validate service override endpoint type Can also be called from the openstack-operator webhook on the HeatSpecCore, like for other operators in [1]. Depends-On: https://github.com/openstack-k8s-operators/lib-common/pull/505 [1] https://github.com/openstack-k8s-operators/openstack-operator/blob/d2703d3a321c979dacaca95b5d4a634bf116e0db/apis/core/v1beta1/openstackcontrolplane_webhook.go#L181 --- api/go.mod | 2 +- api/go.sum | 4 +- api/v1beta1/heat_webhook.go | 115 +++++++++++++++++++++++--- go.mod | 2 +- go.sum | 4 +- tests/functional/heat_webhook_test.go | 66 +++++++++++++++ 6 files changed, 176 insertions(+), 17 deletions(-) diff --git a/api/go.mod b/api/go.mod index cc80999b..405c39cc 100644 --- a/api/go.mod +++ b/api/go.mod @@ -3,7 +3,7 @@ module github.com/openstack-k8s-operators/heat-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 k8s.io/api v0.28.9 k8s.io/apimachinery v0.28.9 sigs.k8s.io/controller-runtime v0.16.5 diff --git a/api/go.sum b/api/go.sum index 614c10ef..e9f8ba71 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/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= diff --git a/api/v1beta1/heat_webhook.go b/api/v1beta1/heat_webhook.go index 5fbef9bc..191d03aa 100644 --- a/api/v1beta1/heat_webhook.go +++ b/api/v1beta1/heat_webhook.go @@ -25,6 +25,7 @@ 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/runtime/schema" @@ -99,10 +100,55 @@ var _ webhook.Validator = &Heat{} func (r *Heat) ValidateCreate() (admission.Warnings, error) { heatlog.Info("validate create", "name", r.Name) - // TODO(user): fill in your validation logic upon object creation. + var 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( + schema.GroupKind{Group: "ironic.openstack.org", Kind: "Ironic"}, + r.Name, allErrs) + } + return nil, nil } +// ValidateCreate - Exported function wrapping non-exported validate functions, +// this function can be called externally to validate an heat spec. +func (r *HeatSpec) ValidateCreate(basePath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + // validate the service override key is valid + allErrs = append(allErrs, service.ValidateRoutedOverrides( + basePath.Child("heatAPI").Child("override").Child("service"), + r.HeatAPI.Override.Service)...) + + // validate the service override key is valid + allErrs = append(allErrs, service.ValidateRoutedOverrides( + basePath.Child("heatCfnAPI").Child("override").Child("service"), + r.HeatCfnAPI.Override.Service)...) + + return allErrs +} + +func (r *HeatSpecCore) ValidateCreate(basePath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + // validate the service override key is valid + allErrs = append(allErrs, service.ValidateRoutedOverrides( + basePath.Child("heatAPI").Child("override").Child("service"), + r.HeatAPI.Override.Service)...) + + // validate the service override key is valid + allErrs = append(allErrs, service.ValidateRoutedOverrides( + basePath.Child("heatCfnAPI").Child("override").Child("service"), + r.HeatCfnAPI.Override.Service)...) + + return allErrs +} + // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type func (r *Heat) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { heatlog.Info("validate update", "name", r.Name) @@ -112,25 +158,72 @@ func (r *Heat) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { fmt.Errorf("Expected a Heatv1 object, but got %T", oldHeat)) } - var errors field.ErrorList + var allErrs field.ErrorList + basePath := field.NewPath("spec") + + if err := r.Spec.ValidateUpdate(oldHeat.Spec, basePath); err != nil { + allErrs = append(allErrs, err...) + } + + if len(allErrs) != 0 { + return nil, apierrors.NewInvalid( + schema.GroupKind{Group: "heat.openstack.org", Kind: "Heat"}, + r.Name, allErrs) + } + + return nil, nil +} + +// ValidateUpdate - Exported function wrapping non-exported validate functions, +// this function can be called externally to validate an barbican spec. +func (r *HeatSpec) ValidateUpdate(old HeatSpec, basePath *field.Path) field.ErrorList { + var allErrs field.ErrorList + // We currently have no logic in place to perform database migrations. Changing databases // would render all of the existing stacks unmanageable. We should block changes to the // databaseInstance to protect existing workloads. - if r.Spec.DatabaseInstance != oldHeat.Spec.DatabaseInstance { - errors = append(errors, field.Forbidden( + if r.DatabaseInstance != old.DatabaseInstance { + allErrs = append(allErrs, field.Forbidden( field.NewPath("spec.databaseInstance"), "Changing the DatabaseInstance is not supported for existing deployments")) } - if errors != nil { - return nil, apierrors.NewInvalid( - schema.GroupKind{Group: "heat.openstack.org", Kind: "Heat"}, - r.Name, - errors, - ) + // validate the service override key is valid + allErrs = append(allErrs, service.ValidateRoutedOverrides( + basePath.Child("heatAPI").Child("override").Child("service"), + r.HeatAPI.Override.Service)...) + + // validate the service override key is valid + allErrs = append(allErrs, service.ValidateRoutedOverrides( + basePath.Child("heatCfnAPI").Child("override").Child("service"), + r.HeatCfnAPI.Override.Service)...) + + return allErrs +} + +func (r *HeatSpecCore) ValidateUpdate(old HeatSpecCore, basePath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + // We currently have no logic in place to perform database migrations. Changing databases + // would render all of the existing stacks unmanageable. We should block changes to the + // databaseInstance to protect existing workloads. + if r.DatabaseInstance != old.DatabaseInstance { + allErrs = append(allErrs, field.Forbidden( + field.NewPath("spec.databaseInstance"), + "Changing the DatabaseInstance is not supported for existing deployments")) } - return nil, nil + // validate the service override key is valid + allErrs = append(allErrs, service.ValidateRoutedOverrides( + basePath.Child("heatAPI").Child("override").Child("service"), + r.HeatAPI.Override.Service)...) + + // validate the service override key is valid + allErrs = append(allErrs, service.ValidateRoutedOverrides( + basePath.Child("heatCfnAPI").Child("override").Child("service"), + r.HeatCfnAPI.Override.Service)...) + + return allErrs } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type diff --git a/go.mod b/go.mod index 335c52dd..aac0abb5 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/openstack-k8s-operators/heat-operator/api v0.3.1-0.20240214134649-6643d1b09d49 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/openstack 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 1efec352..0cacc28a 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/test v0.3.1-0.20240429052447-09a614506ca6 h1:Xx8uGcklRNHgBKTftNcK/Ob3qxiKwxUf5fVjtWCvOHI= diff --git a/tests/functional/heat_webhook_test.go b/tests/functional/heat_webhook_test.go index b3303d79..5d0da680 100644 --- a/tests/functional/heat_webhook_test.go +++ b/tests/functional/heat_webhook_test.go @@ -22,7 +22,9 @@ import ( . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports . "github.com/onsi/gomega" //revive:disable:dot-imports + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" heatv1 "github.com/openstack-k8s-operators/heat-operator/api/v1beta1" ) @@ -104,4 +106,68 @@ var _ = Describe("Heat Webhook", func() { }).Should(ContainSubstring("Changing the DatabaseInstance is not supported for existing deployments")) }) }) + + It("rejects with wrong HeatAPI service override endpoint type", func() { + spec := GetDefaultHeatSpec() + apiSpec := GetDefaultHeatAPISpec() + apiSpec["override"] = map[string]interface{}{ + "service": map[string]interface{}{ + "internal": map[string]interface{}{}, + "wrooong": map[string]interface{}{}, + }, + } + spec["heatAPI"] = apiSpec + + raw := map[string]interface{}{ + "apiVersion": "heat.openstack.org/v1beta1", + "kind": "Heat", + "metadata": map[string]interface{}{ + "name": heatName.Name, + "namespace": heatName.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.heatAPI.override.service[wrooong]: " + + "Invalid value: \"wrooong\": invalid endpoint type: wrooong"), + ) + }) + + It("rejects with wrong HeatCfnAPI service override endpoint type", func() { + spec := GetDefaultHeatSpec() + apiSpec := GetDefaultHeatAPISpec() + apiSpec["override"] = map[string]interface{}{ + "service": map[string]interface{}{ + "internal": map[string]interface{}{}, + "wrooong": map[string]interface{}{}, + }, + } + spec["heatCfnAPI"] = apiSpec + + raw := map[string]interface{}{ + "apiVersion": "heat.openstack.org/v1beta1", + "kind": "Heat", + "metadata": map[string]interface{}{ + "name": heatName.Name, + "namespace": heatName.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.heatCfnAPI.override.service[wrooong]: " + + "Invalid value: \"wrooong\": invalid endpoint type: wrooong"), + ) + }) })