From 1504dcfac0613503bc5f32475c25dee41c5b0317 Mon Sep 17 00:00:00 2001 From: rakeshgm Date: Tue, 16 Jan 2024 12:37:33 +0530 Subject: [PATCH] remove DRPolicy webhook Signed-off-by: rakeshgm --- PROJECT | 3 - api/v1alpha1/drpolicy_types.go | 17 ++++- api/v1alpha1/drpolicy_webhook.go | 73 ------------------- .../ramendr.openshift.io_drpolicies.yaml | 20 ++++- config/crd/patches/webhook_in_drpolicies.yaml | 2 - config/hub/crd/kustomization.yaml | 2 - config/webhook/manifests.yaml | 19 ----- controllers/drpolicy_controller_test.go | 5 +- main.go | 6 -- 9 files changed, 36 insertions(+), 111 deletions(-) delete mode 100644 api/v1alpha1/drpolicy_webhook.go diff --git a/PROJECT b/PROJECT index 8b5032c63c..931a7f7dcd 100644 --- a/PROJECT +++ b/PROJECT @@ -27,9 +27,6 @@ resources: kind: DRPolicy path: github.com/ramendr/ramen/api/v1alpha1 version: v1alpha1 - webhooks: - validation: true - webhookVersion: v1 - api: crdVersion: v1 namespaced: true diff --git a/api/v1alpha1/drpolicy_types.go b/api/v1alpha1/drpolicy_types.go index 23be1e561a..49bf87bebd 100644 --- a/api/v1alpha1/drpolicy_types.go +++ b/api/v1alpha1/drpolicy_types.go @@ -7,7 +7,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// nolint:lll + // DRPolicySpec defines the desired state of DRPolicy +// +kubebuilder:validation:XValidation:rule="!has(oldSelf.replicationClassSelector) || has(self.replicationClassSelector)", message="replicationClassSelector is required once set" +// +kubebuilder:validation:XValidation:rule="!has(oldSelf.volumeSnapshotClassSelector) || has(self.volumeSnapshotClassSelector)", message="volumeSnapshotClassSelector is required once set" type DRPolicySpec struct { // Important: Run "make" to regenerate code after modifying this file @@ -16,22 +20,27 @@ type DRPolicySpec struct { // form . Here is a number, 'm' means // minutes, 'h' means hours and 'd' stands for days. // +kubebuilder:validation:Required - // +kubebuilder:validation:Pattern=`^\d+[mhd]$` - SchedulingInterval string `json:"schedulingInterval,omitempty"` + // +kubebuilder:validation:Pattern=`^(|\d+[mhd])$` + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="schedulingInterval is immutable" + SchedulingInterval string `json:"schedulingInterval"` // Label selector to identify all the VolumeReplicationClasses. // This selector is assumed to be the same for all subscriptions that // need DR protection. It will be passed in to the VRG when it is created - //+optional + // +kubebuilder:validation:Optional + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="replicationClassSelector is immutable" ReplicationClassSelector metav1.LabelSelector `json:"replicationClassSelector,omitempty"` // Label selector to identify all the VolumeSnapshotClasses. // This selector is assumed to be the same for all subscriptions that // need DR protection. It will be passed in to the VRG when it is created - //+optional + // +kubebuilder:validation:Optional + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="volumeSnapshotClassSelector is immutable" VolumeSnapshotClassSelector metav1.LabelSelector `json:"volumeSnapshotClassSelector,omitempty"` // List of DRCluster resources that are governed by this policy + // +kubebuilder:validation:Required + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="drClusters is immutable" DRClusters []string `json:"drClusters"` } diff --git a/api/v1alpha1/drpolicy_webhook.go b/api/v1alpha1/drpolicy_webhook.go deleted file mode 100644 index 4a4fac6c92..0000000000 --- a/api/v1alpha1/drpolicy_webhook.go +++ /dev/null @@ -1,73 +0,0 @@ -// SPDX-FileCopyrightText: The RamenDR authors -// SPDX-License-Identifier: Apache-2.0 - -package v1alpha1 - -import ( - "fmt" - "reflect" - - "k8s.io/apimachinery/pkg/runtime" - ctrl "sigs.k8s.io/controller-runtime" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" -) - -// log is for logging in this package. -var drpolicylog = logf.Log.WithName("drpolicy-resource") - -func (r *DRPolicy) SetupWebhookWithManager(mgr ctrl.Manager) error { - return ctrl.NewWebhookManagedBy(mgr). - For(r). - Complete() -} - -//nolint -//+kubebuilder:webhook:path=/validate-ramendr-openshift-io-v1alpha1-drpolicy,mutating=false,failurePolicy=fail,sideEffects=None,groups=ramendr.openshift.io,resources=drpolicies,verbs=update,versions=v1alpha1,name=vdrpolicy.kb.io,admissionReviewVersions=v1 - -var _ webhook.Validator = &DRPolicy{} - -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type -func (r *DRPolicy) ValidateCreate() (admission.Warnings, error) { - drpolicylog.Info("validate create", "name", r.Name) - - return nil, nil -} - -// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type -func (r *DRPolicy) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { - drpolicylog.Info("validate update", "name", r.Name) - - oldDRPolicy, ok := old.(*DRPolicy) - - if !ok { - return nil, fmt.Errorf("error casting old DRPolicy") - } - - // checks for immutability - if r.Spec.SchedulingInterval != oldDRPolicy.Spec.SchedulingInterval { - return nil, fmt.Errorf("SchedulingInterval cannot be changed") - } - - if !reflect.DeepEqual(r.Spec.ReplicationClassSelector, oldDRPolicy.Spec.ReplicationClassSelector) { - return nil, fmt.Errorf("ReplicationClassSelector cannot be changed") - } - - if !reflect.DeepEqual(r.Spec.VolumeSnapshotClassSelector, oldDRPolicy.Spec.VolumeSnapshotClassSelector) { - return nil, fmt.Errorf("VolumeSnapshotClassSelector cannot be changed") - } - - if !reflect.DeepEqual(r.Spec.DRClusters, oldDRPolicy.Spec.DRClusters) { - return nil, fmt.Errorf("DRClusters cannot be changed") - } - - return nil, nil -} - -// ValidateDelete implements webhook.Validator so a webhook will be registered for the type -func (r *DRPolicy) ValidateDelete() (admission.Warnings, error) { - drpolicylog.Info("validate delete", "name", r.Name) - - return nil, nil -} diff --git a/config/crd/bases/ramendr.openshift.io_drpolicies.yaml b/config/crd/bases/ramendr.openshift.io_drpolicies.yaml index 041d21452e..7a52817db0 100644 --- a/config/crd/bases/ramendr.openshift.io_drpolicies.yaml +++ b/config/crd/bases/ramendr.openshift.io_drpolicies.yaml @@ -45,6 +45,9 @@ spec: items: type: string type: array + x-kubernetes-validations: + - message: drClusters is immutable + rule: self == oldSelf replicationClassSelector: description: |- Label selector to identify all the VolumeReplicationClasses. @@ -92,14 +95,20 @@ spec: type: object type: object x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: replicationClassSelector is immutable + rule: self == oldSelf schedulingInterval: description: |- scheduling Interval for replicating Persistent Volume data to a peer cluster. Interval is typically in the form . Here is a number, 'm' means minutes, 'h' means hours and 'd' stands for days. - pattern: ^\d+[mhd]$ + pattern: ^(|\d+[mhd])$ type: string + x-kubernetes-validations: + - message: schedulingInterval is immutable + rule: self == oldSelf volumeSnapshotClassSelector: description: |- Label selector to identify all the VolumeSnapshotClasses. @@ -147,9 +156,18 @@ spec: type: object type: object x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: volumeSnapshotClassSelector is immutable + rule: self == oldSelf required: - drClusters + - schedulingInterval type: object + x-kubernetes-validations: + - message: replicationClassSelector is required once set + rule: '!has(oldSelf.replicationClassSelector) || has(self.replicationClassSelector)' + - message: volumeSnapshotClassSelector is required once set + rule: '!has(oldSelf.volumeSnapshotClassSelector) || has(self.volumeSnapshotClassSelector)' status: description: |- DRPolicyStatus defines the observed state of DRPolicy diff --git a/config/crd/patches/webhook_in_drpolicies.yaml b/config/crd/patches/webhook_in_drpolicies.yaml index 6611e75e52..1d92161508 100644 --- a/config/crd/patches/webhook_in_drpolicies.yaml +++ b/config/crd/patches/webhook_in_drpolicies.yaml @@ -12,5 +12,3 @@ spec: namespace: system name: webhook-service path: /convert - conversionReviewVersions: - - v1 diff --git a/config/hub/crd/kustomization.yaml b/config/hub/crd/kustomization.yaml index 48c45bd79f..9d6681dd75 100644 --- a/config/hub/crd/kustomization.yaml +++ b/config/hub/crd/kustomization.yaml @@ -10,14 +10,12 @@ resources: patchesStrategicMerge: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. # patches here are for enabling the conversion webhook for each CRD -- ../../crd/patches/webhook_in_drpolicies.yaml - ../../crd/patches/webhook_in_drplacementcontrols.yaml - ../../crd/patches/webhook_in_drclusters.yaml # +kubebuilder:scaffold:crdkustomizewebhookpatch # [CERTMANAGER] To enable webhook, uncomment all the sections with [CERTMANAGER] prefix. # patches here are for enabling the CA injection for each CRD -- ../../crd/patches/cainjection_in_drpolicies.yaml - ../../crd/patches/cainjection_in_drplacementcontrols.yaml - ../../crd/patches/cainjection_in_drclusters.yaml # +kubebuilder:scaffold:crdkustomizecainjectionpatch diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 37d46baece..2878309758 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -43,22 +43,3 @@ webhooks: resources: - drplacementcontrols sideEffects: None -- admissionReviewVersions: - - v1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /validate-ramendr-openshift-io-v1alpha1-drpolicy - failurePolicy: Fail - name: vdrpolicy.kb.io - rules: - - apiGroups: - - ramendr.openshift.io - apiVersions: - - v1alpha1 - operations: - - UPDATE - resources: - - drpolicies - sideEffects: None diff --git a/controllers/drpolicy_controller_test.go b/controllers/drpolicy_controller_test.go index 397fd9320d..23a9cb3d5c 100644 --- a/controllers/drpolicy_controller_test.go +++ b/controllers/drpolicy_controller_test.go @@ -288,7 +288,7 @@ var _ = Describe("DrpolicyController", func() { validationErrors.FailedPattern( path.String(), `body`, - `^\d+[mhd]$`, + `^(|\d+[mhd])$`, value, ).Error(), ), @@ -321,6 +321,9 @@ var _ = Describe("DrpolicyController", func() { path, "", ), + field.Invalid(nil, "null", + "some validation rules were not checked because the object was invalid; "+ + "correct the existing errors to complete validation"), }, ) }() diff --git a/main.go b/main.go index 4d847f2ddc..b5c17fadfe 100644 --- a/main.go +++ b/main.go @@ -213,12 +213,6 @@ func setupReconcilersHub(mgr ctrl.Manager) { os.Exit(1) } - // setup webhook for DRPolicy - if err := (&ramendrv1alpha1.DRPolicy{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "DRPolicy") - os.Exit(1) - } - // setup webhook for drpc if err := (&ramendrv1alpha1.DRPlacementControl{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "DRPlacementControl")