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) <raukadah@gmail.com>

Author: raukadah

api/v1beta1/common_types.go

@@ -62,7 +62,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
@@ -72,7 +72,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

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
 }
 

api/v1beta1/zz_generated.deepcopy.go

@@ -444,11 +444,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
@@ -515,7 +515,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

controllers/watcher_controller.go

@@ -17,6 +17,7 @@ limitations under the License.
 package functional
 
 import (
+	"errors"
 	"fmt"
 
 	. "github.com/onsi/gomega" //revive:disable:dot-imports
@@ -69,6 +70,25 @@ func CreateWatcher(name types.NamespacedName, spec map[string]interface{}) clien
 	return th.CreateUnstructured(raw)
 }
 
+func CreateWatcherWithErrorHandling(name types.NamespacedName, spec map[string]interface{}) (client.Object, error) {
+	raw := map[string]interface{}{
+		"apiVersion": "watcher.openstack.org/v1beta1",
+		"kind":       "Watcher",
+		"metadata": map[string]interface{}{
+			"name":      name.Name,
+			"namespace": name.Namespace,
+		},
+		"spec": spec,
+	}
+
+	obj := th.CreateUnstructured(raw)
+	if obj == nil {
+		return nil, errors.New("failed to create Watcher resource")
+	}
+
+	return obj, nil
+}
+
 func GetWatcher(name types.NamespacedName) *watcherv1.Watcher {
 	instance := &watcherv1.Watcher{}
 	Eventually(func(g Gomega) {

tests/functional/base_test.go

@@ -30,6 +30,14 @@ var (
 		"applierContainerImageURL":        "watcher-applier-custom-image",
 		"decisionengineContainerImageURL": "watcher-decision-engine-custom-image",
 	}
+
+	MinimalWatcherEmptyDatabaseSpec = map[string]interface{}{
+		"databaseInstance": "",
+	}
+
+	MinimalWatcherEmptyRabbitMqSpec = map[string]interface{}{
+		"rabbitMqClusterName": "",
+	}
 )
 
 var _ = Describe("Watcher controller with minimal spec values", func() {
@@ -40,11 +48,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())
 		})
@@ -82,11 +90,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())
 		})
 
@@ -188,7 +196,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}},
 					},
@@ -422,7 +430,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}},
 					},
@@ -494,6 +502,20 @@ var _ = Describe("Watcher controller", func() {
 			Expect(Watcher.Spec.ApplierContainerImageURL).To(Equal("watcher-applier-custom-image-env"))
 		})
 	})
+
+	When("Watcher is created with empty databaseinstance", func() {
+		It("should raise an error for empty databaseInstance", func() {
+			_, err := CreateWatcherWithErrorHandling(watcherTest.Instance, MinimalWatcherEmptyDatabaseSpec)
+			Expect(err).To(HaveOccurred())
+		})
+	})
+
+	When("Watcher is created with empty RabbitMqClusterName", func() {
+		It("should raise an error for empty RabbitMqClusterName", func() {
+			_, err := CreateWatcherWithErrorHandling(watcherTest.Instance, MinimalWatcherEmptyRabbitMqSpec)
+			Expect(err).To(HaveOccurred())
+		})
+	})
 	When("Watcher with non-default values are created", func() {
 		BeforeEach(func() {
 			DeferCleanup(th.DeleteInstance, CreateWatcher(watcherTest.Instance, GetNonDefaultWatcherSpec()))
@@ -502,7 +524,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}},
 					},
@@ -512,12 +534,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() {