Add webhook validation for empty database and rabbitmq

databaseInstance and rabbitMqClusterName are required fields. If an user specify databaseInstance and rabbitMqClusterName field as empty string. The webhook should fail it saying as there cannot be empty. This pr adds the validations for the same. Jira: https://issues.redhat.com/browse/OSPRH-11933 Signed-off-by: Chandan Kumar (raukadah) <[email protected]>
openstack-k8s-operators · Jan 17, 2025 · c3c5bca · c3c5bca
1 parent 2f7815b
commit c3c5bca
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 17 deletions.
diff --git a/api/v1beta1/common_types.go b/api/v1beta1/common_types.go
@@ -67,7 +67,7 @@ type WatcherTemplate struct {
 	// +kubebuilder:default=rabbitmq
 	// RabbitMQ instance name
 	// Needed to request a transportURL that is created and used in Watcher
-	RabbitMqClusterName string `json:"rabbitMqClusterName"`
+	RabbitMqClusterName *string `json:"rabbitMqClusterName"`
 
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=osp-secret
@@ -77,7 +77,7 @@ type WatcherTemplate struct {
 	// +kubebuilder:validation:Required
 	// MariaDB instance name
 	// Required to use the mariadb-operator instance to create the DB and user
-	DatabaseInstance string `json:"databaseInstance"`
+	DatabaseInstance *string `json:"databaseInstance"`
 
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=watcher

diff --git a/api/v1beta1/watcher_webhook.go b/api/v1beta1/watcher_webhook.go
@@ -17,6 +17,8 @@ limitations under the License.
 package v1beta1
 
 import (
+	"errors"
+
 	"k8s.io/apimachinery/pkg/runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -64,13 +66,29 @@ var _ webhook.Validator = &Watcher{}
 func (r *Watcher) ValidateCreate() (admission.Warnings, error) {
 	watcherlog.Info("validate create", "name", r.Name)
 
+	if *r.Spec.DatabaseInstance == "" || r.Spec.DatabaseInstance == nil {
+		return nil, errors.New("databaseInstance field should not be empty")
+	}
+
+	if *r.Spec.RabbitMqClusterName == "" || r.Spec.RabbitMqClusterName == nil {
+		return nil, errors.New("rabbitMqClusterName field should not be empty")
+	}
+
 	return nil, nil
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
 func (r *Watcher) ValidateUpdate(runtime.Object) (admission.Warnings, error) {
 	watcherlog.Info("validate update", "name", r.Name)
 
+	if *r.Spec.DatabaseInstance == "" || r.Spec.DatabaseInstance == nil {
+		return nil, errors.New("databaseInstance field should not be empty")
+	}
+
+	if *r.Spec.RabbitMqClusterName == "" || r.Spec.RabbitMqClusterName == nil {
+		return nil, errors.New("rabbitMqClusterName field should not be empty")
+	}
+
 	return nil, nil
 }
 

diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
diff --git a/controllers/watcher_controller.go b/controllers/watcher_controller.go
@@ -457,11 +457,11 @@ func (r *WatcherReconciler) ensureDB(
 	// create watcher DB instance
 	//
 	db := mariadbv1.NewDatabaseForAccount(
-		instance.Spec.DatabaseInstance, // mariadb/galera service to target
-		watcher.DatabaseName,           // name used in CREATE DATABASE in mariadb
-		watcher.DatabaseCRName,         // CR name for MariaDBDatabase
-		instance.Spec.DatabaseAccount,  // CR name for MariaDBAccount
-		instance.Namespace,             // namespace
+		*instance.Spec.DatabaseInstance, // mariadb/galera service to target
+		watcher.DatabaseName,            // name used in CREATE DATABASE in mariadb
+		watcher.DatabaseCRName,          // CR name for MariaDBDatabase
+		instance.Spec.DatabaseAccount,   // CR name for MariaDBAccount
+		instance.Namespace,              // namespace
 	)
 
 	// create or patch the DB
@@ -528,7 +528,7 @@ func (r *WatcherReconciler) ensureMQ(
 	}
 
 	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, transportURL, func() error {
-		transportURL.Spec.RabbitmqClusterName = instance.Spec.RabbitMqClusterName
+		transportURL.Spec.RabbitmqClusterName = *instance.Spec.RabbitMqClusterName
 
 		err := controllerutil.SetControllerReference(instance, transportURL, r.Scheme)
 		return err

diff --git a/tests/functional/watcher_controller_test.go b/tests/functional/watcher_controller_test.go
@@ -15,10 +15,12 @@ import (
 	mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1"
 	watcherv1beta1 "github.com/openstack-k8s-operators/watcher-operator/api/v1beta1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
 var (
@@ -42,11 +44,11 @@ var _ = Describe("Watcher controller with minimal spec values", func() {
 
 		It("should have the Spec fields defaulted", func() {
 			Watcher := GetWatcher(watcherTest.Instance)
-			Expect(Watcher.Spec.DatabaseInstance).Should(Equal("openstack"))
+			Expect(*(Watcher.Spec.DatabaseInstance)).Should(Equal("openstack"))
 			Expect(Watcher.Spec.DatabaseAccount).Should(Equal("watcher"))
 			Expect(Watcher.Spec.Secret).Should(Equal("osp-secret"))
 			Expect(Watcher.Spec.PasswordSelectors).Should(Equal(watcherv1beta1.PasswordSelector{Service: "WatcherPassword"}))
-			Expect(Watcher.Spec.RabbitMqClusterName).Should(Equal("rabbitmq"))
+			Expect(*(Watcher.Spec.RabbitMqClusterName)).Should(Equal("rabbitmq"))
 			Expect(Watcher.Spec.ServiceUser).Should(Equal("watcher"))
 			Expect(Watcher.Spec.PreserveJobs).Should(BeFalse())
 		})
@@ -84,11 +86,11 @@ var _ = Describe("Watcher controller", func() {
 
 		It("should have the Spec fields defaulted", func() {
 			Watcher := GetWatcher(watcherTest.Instance)
-			Expect(Watcher.Spec.DatabaseInstance).Should(Equal("openstack"))
+			Expect(*(Watcher.Spec.DatabaseInstance)).Should(Equal("openstack"))
 			Expect(Watcher.Spec.DatabaseAccount).Should(Equal("watcher"))
 			Expect(Watcher.Spec.ServiceUser).Should(Equal("watcher"))
 			Expect(Watcher.Spec.Secret).Should(Equal("test-osp-secret"))
-			Expect(Watcher.Spec.RabbitMqClusterName).Should(Equal("rabbitmq"))
+			Expect(*(Watcher.Spec.RabbitMqClusterName)).Should(Equal("rabbitmq"))
 			Expect(Watcher.Spec.PreserveJobs).Should(BeFalse())
 		})
 
@@ -197,7 +199,7 @@ var _ = Describe("Watcher controller", func() {
 				mariadb.DeleteDBService,
 				mariadb.CreateDBService(
 					watcherTest.Instance.Namespace,
-					GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
+					*GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
 					corev1.ServiceSpec{
 						Ports: []corev1.ServicePort{{Port: 3306}},
 					},
@@ -459,7 +461,7 @@ var _ = Describe("Watcher controller", func() {
 				mariadb.DeleteDBService,
 				mariadb.CreateDBService(
 					watcherTest.Instance.Namespace,
-					GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
+					*GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
 					corev1.ServiceSpec{
 						Ports: []corev1.ServicePort{{Port: 3306}},
 					},
@@ -531,6 +533,61 @@ var _ = Describe("Watcher controller", func() {
 			Expect(Watcher.Spec.ApplierContainerImageURL).To(Equal("watcher-applier-custom-image-env"))
 		})
 	})
+
+	When("Watcher rejects when empty databaseinstance is used", func() {
+		It("should raise an error for empty databaseInstance", func() {
+			spec := GetDefaultWatcherAPISpec()
+			spec["databaseInstance"] = ""
+
+			raw := map[string]interface{}{
+				"apiVersion": "watcher.openstack.org/v1beta1",
+				"kind":       "watcher",
+				"metadata": map[string]interface{}{
+					"name":      watcherName.Name,
+					"namespace": watcherName.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(
+					"admission webhook \"vwatcher.kb.io\" denied the request: " +
+						"databaseInstance field should not be empty"),
+			)
+
+		})
+	})
+
+	When("Watcher is created with empty RabbitMqClusterName", func() {
+		It("should raise an error for empty RabbitMqClusterName", func() {
+			spec := GetDefaultWatcherAPISpec()
+			spec["rabbitMqClusterName"] = ""
+
+			raw := map[string]interface{}{
+				"apiVersion": "watcher.openstack.org/v1beta1",
+				"kind":       "watcher",
+				"metadata": map[string]interface{}{
+					"name":      watcherName.Name,
+					"namespace": watcherName.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(
+					"admission webhook \"vwatcher.kb.io\" denied the request: " +
+						"rabbitMqClusterName field should not be empty"),
+			)
+		})
+	})
 	When("Watcher with non-default values are created", func() {
 		BeforeEach(func() {
 			DeferCleanup(th.DeleteInstance, CreateWatcher(watcherTest.Instance, GetNonDefaultWatcherSpec()))
@@ -546,7 +603,7 @@ var _ = Describe("Watcher controller", func() {
 				mariadb.DeleteDBService,
 				mariadb.CreateDBService(
 					watcherTest.Instance.Namespace,
-					GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
+					*GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
 					corev1.ServiceSpec{
 						Ports: []corev1.ServicePort{{Port: 3306}},
 					},
@@ -557,12 +614,12 @@ var _ = Describe("Watcher controller", func() {
 
 		It("should have the Spec fields with the expected values", func() {
 			Watcher := GetWatcher(watcherTest.Instance)
-			Expect(Watcher.Spec.DatabaseInstance).Should(Equal("fakeopenstack"))
+			Expect(*(Watcher.Spec.DatabaseInstance)).Should(Equal("fakeopenstack"))
 			Expect(Watcher.Spec.DatabaseAccount).Should(Equal("watcher"))
 			Expect(Watcher.Spec.ServiceUser).Should(Equal("fakeuser"))
 			Expect(Watcher.Spec.Secret).Should(Equal("test-osp-secret"))
 			Expect(Watcher.Spec.PreserveJobs).Should(BeTrue())
-			Expect(Watcher.Spec.RabbitMqClusterName).Should(Equal("rabbitmq"))
+			Expect(*(Watcher.Spec.RabbitMqClusterName)).Should(Equal("rabbitmq"))
 		})
 
 		It("Should create watcher service with custom values", func() {