Adds MirrorPeer webhooks

This commit adds validating and mutating webhooks for MirrorPeer.

Signed-off-by: Vineet Badrinath <vineetbnath@gmail.com>

PROJECT

@@ -16,4 +16,8 @@ resources:
   kind: MirrorPeer
   path: github.com/red-hat-storage/odf-multicluster-orchestrator/api/v1alpha1
   version: v1alpha1
+  webhooks:
+    defaulting: true
+    validation: true
+    webhookVersion: v1
 version: "3"

addons/token-exchange/agent_mirrorpeer_controller.go

@@ -21,8 +21,6 @@ import (
 	"crypto/sha1"
 	"encoding/hex"
 	"fmt"
-	"regexp"
-
 	replicationv1alpha1 "github.com/csi-addons/volume-replication-operator/api/v1alpha1"
 	obv1alpha1 "github.com/kube-object-storage/lib-bucket-provisioner/pkg/apis/objectbucket.io/v1alpha1"
 	ocsv1 "github.com/openshift/ocs-operator/api/v1"
@@ -225,18 +223,6 @@ func (r *MirrorPeerReconciler) enableCSIAddons(ctx context.Context, namespace st
 	return nil
 }
 
-/*
-  validateSchedulingInterval checks if the scheduling interval is valid and ending with a 'm' or 'h' or 'd'
-  else it will return error.
-*/
-func validateSchedulingInterval(interval string) error {
-	re := regexp.MustCompile(`^\d+[mhd]$`)
-	if re.MatchString(interval) {
-		return nil
-	}
-	return fmt.Errorf("invalid scheduling interval %q. interval should have suffix 'm|h|d' ", interval)
-}
-
 func (r *MirrorPeerReconciler) createVolumeReplicationClass(ctx context.Context, mp *multiclusterv1alpha1.MirrorPeer) []error {
 	scr, err := utils.GetCurrentStorageClusterRef(mp, r.SpokeClusterName)
 	var errs []error
@@ -248,12 +234,6 @@ func (r *MirrorPeerReconciler) createVolumeReplicationClass(ctx context.Context,
 		return errs
 	}
 	for _, interval := range mp.Spec.SchedulingIntervals {
-		err = validateSchedulingInterval(interval)
-		if err != nil {
-			klog.Error(err, "Failed to validate scheduling interval")
-			errs = append(errs, err)
-			continue
-		}
 		params := make(map[string]string)
 		params[MirroringModeKey] = DefaultMirroringMode
 		params[SchedulingIntervalKey] = interval

addons/token-exchange/agent_mirrorpeer_controller_test.go

@@ -48,6 +48,7 @@ var (
 			SchedulingIntervals: []string{"10m", "5m", "30m", "1h", "5m"},
 		},
 	}
+	// Validating webhooks in place won't allow for this to be created
 	mirrorpeer2 = multiclusterv1alpha1.MirrorPeer{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "mirrorpeer-with-invalid-scheduling-intervals",
@@ -173,18 +174,6 @@ func TestMirrorPeerReconcile(t *testing.T) {
 				break
 			}
 		}
-
-		_, err := reconciler.r.Reconcile(ctx, ctrl.Request{
-			NamespacedName: types.NamespacedName{
-				Name: mirrorpeer2.Name,
-			},
-		})
-
-		// On reconciling mirrorpeer2 with invalid scheduling intervals, the err should not be nil (because of validation)
-		// This may be caused by other reconciliation errors too but since both mirrorpeers have same items, it should have been long caught by the checks above !
-		if err == nil {
-			t.Errorf("MirrorPeerReconciler Reconcile() should have failed. Error: %s", err)
-		}
 	}
 
 }

api/v1alpha1/mirrorpeer_webhook.go

@@ -0,0 +1,134 @@
+/*
+Copyright 2021 Red Hat OpenShift Data Foundation.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1alpha1
+
+import (
+	"fmt"
+	"k8s.io/apimachinery/pkg/runtime"
+	"regexp"
+	ctrl "sigs.k8s.io/controller-runtime"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+)
+
+// log is for logging in this package.
+var mirrorpeerlog = logf.Log.WithName("mirrorpeer-webhook")
+
+const (
+	WebhookCertDir  = "/apiserver.local.config/certificates"
+	WebhookCertName = "apiserver.crt"
+	WebhookKeyName  = "apiserver.key"
+)
+
+func (r *MirrorPeer) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	bldr := ctrl.NewWebhookManagedBy(mgr).
+		For(r)
+
+	srv := mgr.GetWebhookServer()
+	srv.CertDir = WebhookCertDir
+	srv.CertName = WebhookCertName
+	srv.KeyName = WebhookKeyName
+
+	return bldr.Complete()
+}
+
+//+kubebuilder:webhook:path=/mutate-multicluster-odf-openshift-io-v1alpha1-mirrorpeer,mutating=true,failurePolicy=fail,sideEffects=None,groups=multicluster.odf.openshift.io,resources=mirrorpeers,verbs=create;update,versions=v1alpha1,name=mmirrorpeer.kb.io,admissionReviewVersions=v1;v1beta1
+
+var _ webhook.Defaulter = &MirrorPeer{}
+
+// Default implements webhook.Defaulter so a webhook will be registered for the type
+func (r *MirrorPeer) Default() {
+	if len(r.Spec.SchedulingIntervals) == 0 {
+		mirrorpeerlog.Info("No values set for schedulingIntervals, defaulting to 5m", "MirrorPeer", r.ObjectMeta.Name)
+		r.Spec.SchedulingIntervals = []string{"5m"}
+	}
+}
+
+//+kubebuilder:webhook:path=/validate-multicluster-odf-openshift-io-v1alpha1-mirrorpeer,mutating=false,failurePolicy=fail,sideEffects=None,groups=multicluster.odf.openshift.io,resources=mirrorpeers,verbs=create;update;delete,versions=v1alpha1,name=vmirrorpeer.kb.io,admissionReviewVersions=v1;v1beta1
+
+var _ webhook.Validator = &MirrorPeer{}
+
+// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
+func (r *MirrorPeer) ValidateCreate() error {
+	mirrorpeerlog.Info("validate create", "name", r.ObjectMeta.Name)
+
+	return validateMirrorPeer(r)
+}
+
+// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
+func (r *MirrorPeer) ValidateUpdate(old runtime.Object) error {
+	mirrorpeerlog.Info("validate update", "name", r.ObjectMeta.Name)
+	oldMirrorPeer, ok := old.(*MirrorPeer)
+	if !ok {
+		return fmt.Errorf("error casting old object to MirrorPeer")
+	}
+
+	if len(r.Spec.Items) != len(oldMirrorPeer.Spec.Items) {
+		return fmt.Errorf("error updating MirrorPeer, new and old spec.items have different lengths")
+	}
+
+	refs := make(map[string]int)
+	for idx, pr := range oldMirrorPeer.Spec.Items {
+		// Creating a make-shift set of references to check for duplicates
+		refs[fmt.Sprintf("%s-%s-%s", pr.ClusterName, pr.StorageClusterRef.Namespace, pr.StorageClusterRef.Name)] = idx
+	}
+
+	for _, pr := range r.Spec.Items {
+		key := fmt.Sprintf("%s-%s-%s", pr.ClusterName, pr.StorageClusterRef.Namespace, pr.StorageClusterRef.Name)
+		if _, ok := refs[key]; !ok {
+			return fmt.Errorf("error validating update: new MirrorPeer %s references a StorageCluster %s/%s that is not in the old MirrorPeer", r.ObjectMeta.Name, pr.StorageClusterRef.Namespace, pr.StorageClusterRef.Name)
+		}
+	}
+	return validateMirrorPeer(r)
+}
+
+/*
+  validateSchedulingInterval checks if the scheduling interval is valid and ending with a 'm' or 'h' or 'd'
+  else it will return error.
+*/
+func validateSchedulingInterval(interval string) error {
+	re := regexp.MustCompile(`^\d+[mhd]$`)
+	if re.MatchString(interval) {
+		return nil
+	}
+	return fmt.Errorf("invalid scheduling interval %q. interval should have suffix 'm|h|d' ", interval)
+}
+
+// validateMirrorPeer validates the MirrorPeer
+func validateMirrorPeer(instance *MirrorPeer) error {
+
+	if instance.Spec.Items == nil {
+		return fmt.Errorf("Spec.Items can not be nil")
+	}
+
+	if instance.Spec.SchedulingIntervals == nil || len(instance.Spec.SchedulingIntervals) == 0 {
+		return fmt.Errorf("Spec.SchedulingIntervals can not be nil or empty")
+	}
+
+	for _, interval := range instance.Spec.SchedulingIntervals {
+		if err := validateSchedulingInterval(interval); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
+func (r *MirrorPeer) ValidateDelete() error {
+	mirrorpeerlog.Info("validate delete", "name", r.ObjectMeta.Name)
+	return nil
+}

bundle/manifests/odf-multicluster-orchestrator.clusterserviceversion.yaml

@@ -213,6 +213,10 @@ spec:
                   initialDelaySeconds: 15
                   periodSeconds: 20
                 name: manager
+                ports:
+                - containerPort: 9443
+                  name: webhook-server
+                  protocol: TCP
                 readinessProbe:
                   httpGet:
                     path: /readyz
@@ -292,3 +296,47 @@ spec:
   provider:
     name: Red Hat
   version: 0.0.1
+  webhookdefinitions:
+  - admissionReviewVersions:
+    - v1
+    - v1beta1
+    containerPort: 443
+    deploymentName: odfmo-controller-manager
+    failurePolicy: Fail
+    generateName: mmirrorpeer.kb.io
+    rules:
+    - apiGroups:
+      - multicluster.odf.openshift.io
+      apiVersions:
+      - v1alpha1
+      operations:
+      - CREATE
+      - UPDATE
+      resources:
+      - mirrorpeers
+    sideEffects: None
+    targetPort: 9443
+    type: MutatingAdmissionWebhook
+    webhookPath: /mutate-multicluster-odf-openshift-io-v1alpha1-mirrorpeer
+  - admissionReviewVersions:
+    - v1
+    - v1beta1
+    containerPort: 443
+    deploymentName: odfmo-controller-manager
+    failurePolicy: Fail
+    generateName: vmirrorpeer.kb.io
+    rules:
+    - apiGroups:
+      - multicluster.odf.openshift.io
+      apiVersions:
+      - v1alpha1
+      operations:
+      - CREATE
+      - UPDATE
+      - DELETE
+      resources:
+      - mirrorpeers
+    sideEffects: None
+    targetPort: 9443
+    type: ValidatingAdmissionWebhook
+    webhookPath: /validate-multicluster-odf-openshift-io-v1alpha1-mirrorpeer

bundle/manifests/odfmo-webhook-service_v1_service.yaml

@@ -0,0 +1,16 @@
+apiVersion: v1
+kind: Service
+metadata:
+  creationTimestamp: null
+  labels:
+    control-plane: odfmo-controller-manager
+  name: odfmo-webhook-service
+spec:
+  ports:
+  - port: 443
+    protocol: TCP
+    targetPort: 9443
+  selector:
+    control-plane: odfmo-controller-manager
+status:
+  loadBalancer: {}

config/certmanager/certificate.yaml

@@ -0,0 +1,25 @@
+# The following manifests contain a self-signed issuer CR and a certificate CR.
+# More document can be found at https://docs.cert-manager.io
+# WARNING: Targets CertManager v1.0. Check https://cert-manager.io/docs/installation/upgrading/ for breaking changes.
+apiVersion: cert-manager.io/v1
+kind: Issuer
+metadata:
+  name: selfsigned-issuer
+  namespace: system
+spec:
+  selfSigned: {}
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: serving-cert  # this name should match the one appeared in kustomizeconfig.yaml
+  namespace: system
+spec:
+  # $(SERVICE_NAME) and $(SERVICE_NAMESPACE) will be substituted by kustomize
+  dnsNames:
+  - $(SERVICE_NAME).$(SERVICE_NAMESPACE).svc
+  - $(SERVICE_NAME).$(SERVICE_NAMESPACE).svc.cluster.local
+  issuerRef:
+    kind: Issuer
+    name: selfsigned-issuer
+  secretName: webhook-server-cert # this secret will not be prefixed, since it's not managed by kustomize

config/certmanager/kustomization.yaml

@@ -0,0 +1,5 @@
+resources:
+- certificate.yaml
+
+configurations:
+- kustomizeconfig.yaml

config/certmanager/kustomizeconfig.yaml

@@ -0,0 +1,16 @@
+# This configuration is for teaching kustomize how to update name ref and var substitution 
+nameReference:
+- kind: Issuer
+  group: cert-manager.io
+  fieldSpecs:
+  - kind: Certificate
+    group: cert-manager.io
+    path: spec/issuerRef/name
+
+varReference:
+- kind: Certificate
+  group: cert-manager.io
+  path: spec/commonName
+- kind: Certificate
+  group: cert-manager.io
+  path: spec/dnsNames

config/default/kustomization.yaml

@@ -18,7 +18,7 @@ bases:
 - ../manager
 # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
 # crd/kustomization.yaml
-#- ../webhook
+- ../webhook
 # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required.
 #- ../certmanager
 # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'.
@@ -36,7 +36,7 @@ patchesStrategicMerge:
 
 # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
 # crd/kustomization.yaml
-#- manager_webhook_patch.yaml
+- manager_webhook_patch.yaml
 
 # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'.
 # Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks.

config/default/manager_webhook_patch.yaml

@@ -0,0 +1,23 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: controller-manager
+  namespace: system
+spec:
+  template:
+    spec:
+      containers:
+      - name: manager
+        ports:
+        - containerPort: 9443
+          name: webhook-server
+          protocol: TCP
+        volumeMounts:
+        - mountPath: /tmp/k8s-webhook-server/serving-certs
+          name: cert
+          readOnly: true
+      volumes:
+      - name: cert
+        secret:
+          defaultMode: 420
+          secretName: webhook-server-cert