Skip to content

Commit 90c6e85

Browse files
raukadahraukadah
committed
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>
1 parent a22488f commit 90c6e85

File tree

5 files changed

+104
-19
lines changed

5 files changed

+104
-19
lines changed

‎api/v1beta1/common_types.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ type WatcherTemplate struct {
6262
// +kubebuilder:default=rabbitmq
6363
// RabbitMQ instance name
6464
// Needed to request a transportURL that is created and used in Watcher
65-
RabbitMqClusterName string `json:"rabbitMqClusterName"`
65+
RabbitMqClusterName *string `json:"rabbitMqClusterName"`
6666

6767
// +kubebuilder:validation:Optional
6868
// +kubebuilder:default=osp-secret
@@ -72,7 +72,7 @@ type WatcherTemplate struct {
7272
// +kubebuilder:validation:Required
7373
// MariaDB instance name
7474
// Required to use the mariadb-operator instance to create the DB and user
75-
DatabaseInstance string `json:"databaseInstance"`
75+
DatabaseInstance *string `json:"databaseInstance"`
7676

7777
// +kubebuilder:validation:Optional
7878
// +kubebuilder:default=watcher

‎api/v1beta1/watcher_webhook.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ limitations under the License.
1717
package v1beta1
1818

1919
import (
20+
"errors"
21+
2022
"k8s.io/apimachinery/pkg/runtime"
2123
logf "sigs.k8s.io/controller-runtime/pkg/log"
2224
"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -64,13 +66,29 @@ var _ webhook.Validator = &Watcher{}
6466
func (r *Watcher) ValidateCreate() (admission.Warnings, error) {
6567
watcherlog.Info("validate create", "name", r.Name)
6668

69+
if *r.Spec.DatabaseInstance == "" || r.Spec.DatabaseInstance == nil {
70+
return nil, errors.New("databaseInstance field should not be empty")
71+
}
72+
73+
if *r.Spec.RabbitMqClusterName == "" || r.Spec.RabbitMqClusterName == nil {
74+
return nil, errors.New("rabbitMqClusterName field should not be empty")
75+
}
76+
6777
return nil, nil
6878
}
6979

7080
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
7181
func (r *Watcher) ValidateUpdate(runtime.Object) (admission.Warnings, error) {
7282
watcherlog.Info("validate update", "name", r.Name)
7383

84+
if *r.Spec.DatabaseInstance == "" || r.Spec.DatabaseInstance == nil {
85+
return nil, errors.New("databaseInstance field should not be empty")
86+
}
87+
88+
if *r.Spec.RabbitMqClusterName == "" || r.Spec.RabbitMqClusterName == nil {
89+
return nil, errors.New("rabbitMqClusterName field should not be empty")
90+
}
91+
7492
return nil, nil
7593
}
7694

‎api/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 12 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎controllers/watcher_controller.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -444,11 +444,11 @@ func (r *WatcherReconciler) ensureDB(
444444
// create watcher DB instance
445445
//
446446
db := mariadbv1.NewDatabaseForAccount(
447-
instance.Spec.DatabaseInstance, // mariadb/galera service to target
448-
watcher.DatabaseName, // name used in CREATE DATABASE in mariadb
449-
watcher.DatabaseCRName, // CR name for MariaDBDatabase
450-
instance.Spec.DatabaseAccount, // CR name for MariaDBAccount
451-
instance.Namespace, // namespace
447+
*instance.Spec.DatabaseInstance, // mariadb/galera service to target
448+
watcher.DatabaseName, // name used in CREATE DATABASE in mariadb
449+
watcher.DatabaseCRName, // CR name for MariaDBDatabase
450+
instance.Spec.DatabaseAccount, // CR name for MariaDBAccount
451+
instance.Namespace, // namespace
452452
)
453453

454454
// create or patch the DB
@@ -515,7 +515,7 @@ func (r *WatcherReconciler) ensureMQ(
515515
}
516516

517517
op, err := controllerutil.CreateOrUpdate(ctx, r.Client, transportURL, func() error {
518-
transportURL.Spec.RabbitmqClusterName = instance.Spec.RabbitMqClusterName
518+
transportURL.Spec.RabbitmqClusterName = *instance.Spec.RabbitMqClusterName
519519

520520
err := controllerutil.SetControllerReference(instance, transportURL, r.Scheme)
521521
return err

‎tests/functional/watcher_controller_test.go‎

Lines changed: 66 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@ import (
1414
mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1"
1515
watcherv1beta1 "github.com/openstack-k8s-operators/watcher-operator/api/v1beta1"
1616
corev1 "k8s.io/api/core/v1"
17+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1718
"k8s.io/apimachinery/pkg/fields"
1819
"k8s.io/apimachinery/pkg/types"
1920
"sigs.k8s.io/controller-runtime/pkg/client"
21+
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
2022
)
2123

2224
var (
@@ -40,11 +42,11 @@ var _ = Describe("Watcher controller with minimal spec values", func() {
4042

4143
It("should have the Spec fields defaulted", func() {
4244
Watcher := GetWatcher(watcherTest.Instance)
43-
Expect(Watcher.Spec.DatabaseInstance).Should(Equal("openstack"))
45+
Expect(*(Watcher.Spec.DatabaseInstance)).Should(Equal("openstack"))
4446
Expect(Watcher.Spec.DatabaseAccount).Should(Equal("watcher"))
4547
Expect(Watcher.Spec.Secret).Should(Equal("osp-secret"))
4648
Expect(Watcher.Spec.PasswordSelectors).Should(Equal(watcherv1beta1.PasswordSelector{Service: "WatcherPassword"}))
47-
Expect(Watcher.Spec.RabbitMqClusterName).Should(Equal("rabbitmq"))
49+
Expect(*(Watcher.Spec.RabbitMqClusterName)).Should(Equal("rabbitmq"))
4850
Expect(Watcher.Spec.ServiceUser).Should(Equal("watcher"))
4951
Expect(Watcher.Spec.PreserveJobs).Should(BeFalse())
5052
})
@@ -82,11 +84,11 @@ var _ = Describe("Watcher controller", func() {
8284

8385
It("should have the Spec fields defaulted", func() {
8486
Watcher := GetWatcher(watcherTest.Instance)
85-
Expect(Watcher.Spec.DatabaseInstance).Should(Equal("openstack"))
87+
Expect(*(Watcher.Spec.DatabaseInstance)).Should(Equal("openstack"))
8688
Expect(Watcher.Spec.DatabaseAccount).Should(Equal("watcher"))
8789
Expect(Watcher.Spec.ServiceUser).Should(Equal("watcher"))
8890
Expect(Watcher.Spec.Secret).Should(Equal("test-osp-secret"))
89-
Expect(Watcher.Spec.RabbitMqClusterName).Should(Equal("rabbitmq"))
91+
Expect(*(Watcher.Spec.RabbitMqClusterName)).Should(Equal("rabbitmq"))
9092
Expect(Watcher.Spec.PreserveJobs).Should(BeFalse())
9193
})
9294

@@ -188,7 +190,7 @@ var _ = Describe("Watcher controller", func() {
188190
mariadb.DeleteDBService,
189191
mariadb.CreateDBService(
190192
watcherTest.Instance.Namespace,
191-
GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
193+
*GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
192194
corev1.ServiceSpec{
193195
Ports: []corev1.ServicePort{{Port: 3306}},
194196
},
@@ -422,7 +424,7 @@ var _ = Describe("Watcher controller", func() {
422424
mariadb.DeleteDBService,
423425
mariadb.CreateDBService(
424426
watcherTest.Instance.Namespace,
425-
GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
427+
*GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
426428
corev1.ServiceSpec{
427429
Ports: []corev1.ServicePort{{Port: 3306}},
428430
},
@@ -494,6 +496,61 @@ var _ = Describe("Watcher controller", func() {
494496
Expect(Watcher.Spec.ApplierContainerImageURL).To(Equal("watcher-applier-custom-image-env"))
495497
})
496498
})
499+
500+
When("Watcher rejects when empty databaseinstance is used", func() {
501+
It("should raise an error for empty databaseInstance", func() {
502+
spec := GetDefaultWatcherAPISpec()
503+
spec["databaseInstance"] = ""
504+
505+
raw := map[string]interface{}{
506+
"apiVersion": "watcher.openstack.org/v1beta1",
507+
"kind": "watcher",
508+
"metadata": map[string]interface{}{
509+
"name": watcherName.Name,
510+
"namespace": watcherName.Namespace,
511+
},
512+
"spec": spec,
513+
}
514+
515+
unstructuredObj := &unstructured.Unstructured{Object: raw}
516+
_, err := controllerutil.CreateOrPatch(
517+
th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil })
518+
Expect(err).To(HaveOccurred())
519+
Expect(err.Error()).To(
520+
ContainSubstring(
521+
"admission webhook \"vwatcher.kb.io\" denied the request: " +
522+
"databaseInstance field should not be empty"),
523+
)
524+
525+
})
526+
})
527+
528+
When("Watcher is created with empty RabbitMqClusterName", func() {
529+
It("should raise an error for empty RabbitMqClusterName", func() {
530+
spec := GetDefaultWatcherAPISpec()
531+
spec["rabbitMqClusterName"] = ""
532+
533+
raw := map[string]interface{}{
534+
"apiVersion": "watcher.openstack.org/v1beta1",
535+
"kind": "watcher",
536+
"metadata": map[string]interface{}{
537+
"name": watcherName.Name,
538+
"namespace": watcherName.Namespace,
539+
},
540+
"spec": spec,
541+
}
542+
543+
unstructuredObj := &unstructured.Unstructured{Object: raw}
544+
_, err := controllerutil.CreateOrPatch(
545+
th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil })
546+
Expect(err).To(HaveOccurred())
547+
Expect(err.Error()).To(
548+
ContainSubstring(
549+
"admission webhook \"vwatcher.kb.io\" denied the request: " +
550+
"rabbitMqClusterName field should not be empty"),
551+
)
552+
})
553+
})
497554
When("Watcher with non-default values are created", func() {
498555
BeforeEach(func() {
499556
DeferCleanup(th.DeleteInstance, CreateWatcher(watcherTest.Instance, GetNonDefaultWatcherSpec()))
@@ -502,7 +559,7 @@ var _ = Describe("Watcher controller", func() {
502559
mariadb.DeleteDBService,
503560
mariadb.CreateDBService(
504561
watcherTest.Instance.Namespace,
505-
GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
562+
*GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
506563
corev1.ServiceSpec{
507564
Ports: []corev1.ServicePort{{Port: 3306}},
508565
},
@@ -512,12 +569,12 @@ var _ = Describe("Watcher controller", func() {
512569

513570
It("should have the Spec fields with the expected values", func() {
514571
Watcher := GetWatcher(watcherTest.Instance)
515-
Expect(Watcher.Spec.DatabaseInstance).Should(Equal("fakeopenstack"))
572+
Expect(*(Watcher.Spec.DatabaseInstance)).Should(Equal("fakeopenstack"))
516573
Expect(Watcher.Spec.DatabaseAccount).Should(Equal("watcher"))
517574
Expect(Watcher.Spec.ServiceUser).Should(Equal("fakeuser"))
518575
Expect(Watcher.Spec.Secret).Should(Equal("test-osp-secret"))
519576
Expect(Watcher.Spec.PreserveJobs).Should(BeTrue())
520-
Expect(Watcher.Spec.RabbitMqClusterName).Should(Equal("rabbitmq"))
577+
Expect(*(Watcher.Spec.RabbitMqClusterName)).Should(Equal("rabbitmq"))
521578
})
522579

523580
It("Should create watcher service with custom values", func() {

0 commit comments

Comments
 (0)