diff --git a/api/disaggregated/v1/disaggregatedcluster_webhook.go b/api/disaggregated/v1/disaggregatedcluster_webhook.go index 9676d6b3..bf2098d8 100644 --- a/api/disaggregated/v1/disaggregatedcluster_webhook.go +++ b/api/disaggregated/v1/disaggregatedcluster_webhook.go @@ -34,6 +34,8 @@ import ( func (ddc *DorisDisaggregatedCluster) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). For(ddc). + WithDefaulter(ddc). + WithValidator(ddc). Complete() } @@ -53,9 +55,13 @@ var _ webhook.CustomValidator = &DorisDisaggregatedCluster{} // ValidateCreate implements webhook.Validator so a unnamedwatches will be registered for the type func (ddc *DorisDisaggregatedCluster) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { - klog.Info("validate create", "name", ddc.Name) + cluster, ok := obj.(*DorisDisaggregatedCluster) + if !ok { + return nil, fmt.Errorf("expected a DorisDisaggregatedCluster but got %T", obj) + } + klog.Info("validate create", "name", cluster.Name) - if errs := ddc.validate(); len(errs) != 0 { + if errs := cluster.validate(); len(errs) != 0 { return nil, kerrors.NewAggregate(errs) } @@ -64,9 +70,13 @@ func (ddc *DorisDisaggregatedCluster) ValidateCreate(ctx context.Context, obj ru // ValidateUpdate implements webhook.Validator so a unnamedwatches will be registered for the type func (ddc *DorisDisaggregatedCluster) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { - klog.Info("validate update", "name", ddc.Name) + cluster, ok := newObj.(*DorisDisaggregatedCluster) + if !ok { + return nil, fmt.Errorf("expected a DorisDisaggregatedCluster but got %T", newObj) + } + klog.Info("validate update", "name", cluster.Name) - if errs := ddc.validate(); len(errs) != 0 { + if errs := cluster.validate(); len(errs) != 0 { return nil, kerrors.NewAggregate(errs) } diff --git a/api/disaggregated/v1/disaggregatedcluster_webhook_test.go b/api/disaggregated/v1/disaggregatedcluster_webhook_test.go index ff1c2e85..2f604454 100644 --- a/api/disaggregated/v1/disaggregatedcluster_webhook_test.go +++ b/api/disaggregated/v1/disaggregatedcluster_webhook_test.go @@ -23,38 +23,41 @@ import ( ) func TestDorisDisaggregatedClusterRejectsAdminManagementUser(t *testing.T) { + validator := &DorisDisaggregatedCluster{} cluster := &DorisDisaggregatedCluster{ Spec: DorisDisaggregatedClusterSpec{ AdminUser: &AdminUser{Name: "admin"}, }, } - if _, err := cluster.ValidateCreate(context.Background(), cluster); err == nil { + if _, err := validator.ValidateCreate(context.Background(), cluster); err == nil { t.Fatal("expected admin management user to be rejected on create") } - if _, err := cluster.ValidateUpdate(context.Background(), cluster, cluster); err == nil { + if _, err := validator.ValidateUpdate(context.Background(), cluster, cluster); err == nil { t.Fatal("expected admin management user to be rejected on update") } } func TestDorisDisaggregatedClusterAllowsNonAdminManagementUser(t *testing.T) { + validator := &DorisDisaggregatedCluster{} cluster := &DorisDisaggregatedCluster{ Spec: DorisDisaggregatedClusterSpec{ AdminUser: &AdminUser{Name: "doris_operator"}, }, } - if _, err := cluster.ValidateCreate(context.Background(), cluster); err != nil { + if _, err := validator.ValidateCreate(context.Background(), cluster); err != nil { t.Fatalf("expected non-admin management user to be allowed on create: %v", err) } - if _, err := cluster.ValidateUpdate(context.Background(), cluster, cluster); err != nil { + if _, err := validator.ValidateUpdate(context.Background(), cluster, cluster); err != nil { t.Fatalf("expected non-admin management user to be allowed on update: %v", err) } } func TestDorisDisaggregatedClusterValidateFEReplicas(t *testing.T) { + validator := &DorisDisaggregatedCluster{} replicas := int32(1) electionNumber := int32(2) ddc := &DorisDisaggregatedCluster{ @@ -68,18 +71,18 @@ func TestDorisDisaggregatedClusterValidateFEReplicas(t *testing.T) { }, } - if _, err := ddc.ValidateCreate(context.Background(), ddc); err == nil { + if _, err := validator.ValidateCreate(context.Background(), ddc); err == nil { t.Fatal("expected create validation to reject replicas smaller than electionNumber") } - if _, err := ddc.ValidateUpdate(context.Background(), ddc, ddc); err == nil { + if _, err := validator.ValidateUpdate(context.Background(), ddc, ddc); err == nil { t.Fatal("expected update validation to reject replicas smaller than electionNumber") } replicas = 2 - if _, err := ddc.ValidateCreate(context.Background(), ddc); err != nil { + if _, err := validator.ValidateCreate(context.Background(), ddc); err != nil { t.Fatalf("expected valid create to pass, got %v", err) } - if _, err := ddc.ValidateUpdate(context.Background(), ddc, ddc); err != nil { + if _, err := validator.ValidateUpdate(context.Background(), ddc, ddc); err != nil { t.Fatalf("expected valid update to pass, got %v", err) } } diff --git a/api/doris/v1/doriscluster_webhook.go b/api/doris/v1/doriscluster_webhook.go index d1c745e5..a2bf40f6 100644 --- a/api/doris/v1/doriscluster_webhook.go +++ b/api/doris/v1/doriscluster_webhook.go @@ -50,6 +50,8 @@ import ( func (r *DorisCluster) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). For(r). + WithDefaulter(r). + WithValidator(r). Complete() } @@ -70,9 +72,13 @@ var _ webhook.CustomValidator = &DorisCluster{} // ValidateCreate implements webhook.Validator so a unnamedwatches will be registered for the type func (r *DorisCluster) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { - klog.Info("validate create", "name", r.Name) + cluster, ok := obj.(*DorisCluster) + if !ok { + return nil, fmt.Errorf("expected a DorisCluster but got %T", obj) + } + klog.Info("validate create", "name", cluster.Name) - if errs := r.validateManagementUser(); len(errs) != 0 { + if errs := cluster.validateManagementUser(); len(errs) != 0 { return nil, kerrors.NewAggregate(errs) } @@ -81,11 +87,15 @@ func (r *DorisCluster) ValidateCreate(ctx context.Context, obj runtime.Object) ( // ValidateUpdate implements webhook.Validator so a unnamedwatches will be registered for the type func (r *DorisCluster) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { - klog.Info("validate update", "name", r.Name) + cluster, ok := newObj.(*DorisCluster) + if !ok { + return nil, fmt.Errorf("expected a DorisCluster but got %T", newObj) + } + klog.Info("validate update", "name", cluster.Name) var errors []error - errors = append(errors, r.validateManagementUser()...) + errors = append(errors, cluster.validateManagementUser()...) // fe FeSpec.Replicas must greater than or equal to FeSpec.ElectionNumber - if *r.Spec.FeSpec.Replicas < r.GetElectionNumber() { + if cluster.Spec.FeSpec.Replicas != nil && *cluster.Spec.FeSpec.Replicas < cluster.GetElectionNumber() { errors = append(errors, fmt.Errorf("'FeSpec.Replicas' error: the number of FeSpec.Replicas should greater than or equal to FeSpec.ElectionNumber")) } diff --git a/api/doris/v1/doriscluster_webhook_test.go b/api/doris/v1/doriscluster_webhook_test.go index 8298b587..079f8b8e 100644 --- a/api/doris/v1/doriscluster_webhook_test.go +++ b/api/doris/v1/doriscluster_webhook_test.go @@ -23,37 +23,39 @@ import ( ) func TestDorisClusterRejectsAdminManagementUser(t *testing.T) { + validator := &DorisCluster{} cluster := &DorisCluster{ Spec: DorisClusterSpec{ AdminUser: &AdminUser{Name: "admin"}, }, } - if _, err := cluster.ValidateCreate(context.Background(), cluster); err == nil { + if _, err := validator.ValidateCreate(context.Background(), cluster); err == nil { t.Fatal("expected admin management user to be rejected on create") } replicas := int32(3) cluster.Spec.FeSpec = &FeSpec{BaseSpec: BaseSpec{Replicas: &replicas}} - if _, err := cluster.ValidateUpdate(context.Background(), cluster, cluster); err == nil { + if _, err := validator.ValidateUpdate(context.Background(), cluster, cluster); err == nil { t.Fatal("expected admin management user to be rejected on update") } } func TestDorisClusterAllowsNonAdminManagementUser(t *testing.T) { + validator := &DorisCluster{} cluster := &DorisCluster{ Spec: DorisClusterSpec{ AdminUser: &AdminUser{Name: "doris_operator"}, }, } - if _, err := cluster.ValidateCreate(context.Background(), cluster); err != nil { + if _, err := validator.ValidateCreate(context.Background(), cluster); err != nil { t.Fatalf("expected non-admin management user to be allowed on create: %v", err) } replicas := int32(3) cluster.Spec.FeSpec = &FeSpec{BaseSpec: BaseSpec{Replicas: &replicas}} - if _, err := cluster.ValidateUpdate(context.Background(), cluster, cluster); err != nil { + if _, err := validator.ValidateUpdate(context.Background(), cluster, cluster); err != nil { t.Fatalf("expected non-admin management user to be allowed on update: %v", err) } }