Skip to content

Commit 2fe1ea4

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 2fe1ea4

File tree

5 files changed

+75
-19
lines changed

5 files changed

+75
-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: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ var (
3030
"applierContainerImageURL": "watcher-applier-custom-image",
3131
"decisionengineContainerImageURL": "watcher-decision-engine-custom-image",
3232
}
33+
34+
MinimalWatcherEmptyDatabaseSpec = map[string]interface{}{
35+
"databaseInstance": "",
36+
}
37+
38+
MinimalWatcherEmptyRabbitMqSpec = map[string]interface{}{
39+
"rabbitMqClusterName": "",
40+
}
3341
)
3442

3543
var _ = Describe("Watcher controller with minimal spec values", func() {
@@ -40,11 +48,11 @@ var _ = Describe("Watcher controller with minimal spec values", func() {
4048

4149
It("should have the Spec fields defaulted", func() {
4250
Watcher := GetWatcher(watcherTest.Instance)
43-
Expect(Watcher.Spec.DatabaseInstance).Should(Equal("openstack"))
51+
Expect(*(Watcher.Spec.DatabaseInstance)).Should(Equal("openstack"))
4452
Expect(Watcher.Spec.DatabaseAccount).Should(Equal("watcher"))
4553
Expect(Watcher.Spec.Secret).Should(Equal("osp-secret"))
4654
Expect(Watcher.Spec.PasswordSelectors).Should(Equal(watcherv1beta1.PasswordSelector{Service: "WatcherPassword"}))
47-
Expect(Watcher.Spec.RabbitMqClusterName).Should(Equal("rabbitmq"))
55+
Expect(*(Watcher.Spec.RabbitMqClusterName)).Should(Equal("rabbitmq"))
4856
Expect(Watcher.Spec.ServiceUser).Should(Equal("watcher"))
4957
Expect(Watcher.Spec.PreserveJobs).Should(BeFalse())
5058
})
@@ -82,11 +90,11 @@ var _ = Describe("Watcher controller", func() {
8290

8391
It("should have the Spec fields defaulted", func() {
8492
Watcher := GetWatcher(watcherTest.Instance)
85-
Expect(Watcher.Spec.DatabaseInstance).Should(Equal("openstack"))
93+
Expect(*(Watcher.Spec.DatabaseInstance)).Should(Equal("openstack"))
8694
Expect(Watcher.Spec.DatabaseAccount).Should(Equal("watcher"))
8795
Expect(Watcher.Spec.ServiceUser).Should(Equal("watcher"))
8896
Expect(Watcher.Spec.Secret).Should(Equal("test-osp-secret"))
89-
Expect(Watcher.Spec.RabbitMqClusterName).Should(Equal("rabbitmq"))
97+
Expect(*(Watcher.Spec.RabbitMqClusterName)).Should(Equal("rabbitmq"))
9098
Expect(Watcher.Spec.PreserveJobs).Should(BeFalse())
9199
})
92100

@@ -188,7 +196,7 @@ var _ = Describe("Watcher controller", func() {
188196
mariadb.DeleteDBService,
189197
mariadb.CreateDBService(
190198
watcherTest.Instance.Namespace,
191-
GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
199+
*GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
192200
corev1.ServiceSpec{
193201
Ports: []corev1.ServicePort{{Port: 3306}},
194202
},
@@ -422,7 +430,7 @@ var _ = Describe("Watcher controller", func() {
422430
mariadb.DeleteDBService,
423431
mariadb.CreateDBService(
424432
watcherTest.Instance.Namespace,
425-
GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
433+
*GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
426434
corev1.ServiceSpec{
427435
Ports: []corev1.ServicePort{{Port: 3306}},
428436
},
@@ -494,6 +502,26 @@ var _ = Describe("Watcher controller", func() {
494502
Expect(Watcher.Spec.ApplierContainerImageURL).To(Equal("watcher-applier-custom-image-env"))
495503
})
496504
})
505+
506+
When("Watcher is created with empty databaseinstance", func() {
507+
BeforeEach(func() {
508+
DeferCleanup(th.DeleteInstance, CreateWatcher(watcherTest.Instance, MinimalWatcherEmptyDatabaseSpec))
509+
})
510+
It("It should raise error for empty databaseInstance", func() {
511+
err := GetWatcher(watcherTest.Instance)
512+
Expect(err).To(HaveOccurred())
513+
})
514+
})
515+
516+
When("Watcher is created with empty RabbitMqClusterName", func() {
517+
BeforeEach(func() {
518+
DeferCleanup(th.DeleteInstance, CreateWatcher(watcherTest.Instance, MinimalWatcherEmptyRabbitMqSpec))
519+
})
520+
It("It should raise error for empty rabbitMqClusterName", func() {
521+
err := GetWatcher(watcherTest.Instance)
522+
Expect(err).To(HaveOccurred())
523+
})
524+
})
497525
When("Watcher with non-default values are created", func() {
498526
BeforeEach(func() {
499527
DeferCleanup(th.DeleteInstance, CreateWatcher(watcherTest.Instance, GetNonDefaultWatcherSpec()))
@@ -502,7 +530,7 @@ var _ = Describe("Watcher controller", func() {
502530
mariadb.DeleteDBService,
503531
mariadb.CreateDBService(
504532
watcherTest.Instance.Namespace,
505-
GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
533+
*GetWatcher(watcherTest.Instance).Spec.DatabaseInstance,
506534
corev1.ServiceSpec{
507535
Ports: []corev1.ServicePort{{Port: 3306}},
508536
},
@@ -512,12 +540,12 @@ var _ = Describe("Watcher controller", func() {
512540

513541
It("should have the Spec fields with the expected values", func() {
514542
Watcher := GetWatcher(watcherTest.Instance)
515-
Expect(Watcher.Spec.DatabaseInstance).Should(Equal("fakeopenstack"))
543+
Expect(*(Watcher.Spec.DatabaseInstance)).Should(Equal("fakeopenstack"))
516544
Expect(Watcher.Spec.DatabaseAccount).Should(Equal("watcher"))
517545
Expect(Watcher.Spec.ServiceUser).Should(Equal("fakeuser"))
518546
Expect(Watcher.Spec.Secret).Should(Equal("test-osp-secret"))
519547
Expect(Watcher.Spec.PreserveJobs).Should(BeTrue())
520-
Expect(Watcher.Spec.RabbitMqClusterName).Should(Equal("rabbitmq"))
548+
Expect(*(Watcher.Spec.RabbitMqClusterName)).Should(Equal("rabbitmq"))
521549
})
522550

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

0 commit comments

Comments
 (0)