Vm webhook limits (#477)

* Add webhook to add limits to VMs

* Refactoring and tests

* Update failure policies

* Add comments

* Use VM apis from toolchain-common

* Update api dependency

* Fix unit test

* Switch to unstructured type

* Fix comment

* Fix lint errors

---------

Co-authored-by: Francisc Munteanu <fmuntean@redhat.com>
Co-authored-by: Alexey Kazakov <alkazako@redhat.com>

cmd/webhook/main.go

@@ -100,7 +100,8 @@ func main() {
 	}
 	mux := http.NewServeMux()
 
-	mux.HandleFunc("/mutate-users-pods", mutatingwebhook.HandleMutate)
+	mux.HandleFunc("/mutate-users-pods", mutatingwebhook.HandleMutateUserPods)
+	mux.HandleFunc("/mutate-virtual-machines", mutatingwebhook.HandleMutateVirtualMachines)
 	mux.HandleFunc("/validate-users-rolebindings", rolebindingValidator.HandleValidate)
 	mux.HandleFunc("/validate-users-checlusters", checlusterValidator.HandleValidate)
 	mux.HandleFunc("/validate-spacebindingrequests", spacebindingrequestValidator.HandleValidate)

deploy/webhook/member-operator-webhook.yaml

@@ -36,6 +36,14 @@ objects:
       - get
       - list
       - watch
+    - apiGroups:
+      - "kubevirt.io"
+      resources:
+      - "virtualmachines"
+      verbs:
+      - get
+      - list
+      - watch
 - apiVersion: v1
   kind: ServiceAccount
   metadata:
@@ -137,6 +145,33 @@ objects:
     namespaceSelector:
       matchLabels:
         toolchain.dev.openshift.com/provider: codeready-toolchain
+  # The users.virtualmachines.webhook.sandbox webhook sets resource limits on VirtualMachines prior to creation as a workaround for https://issues.redhat.com/browse/CNV-28746 (https://issues.redhat.com/browse/CNV-32069)
+  # This webhook should be updated to remove the workaround once https://issues.redhat.com/browse/CNV-32069 is complete.
+  # The webhook code is available at member-operator/pkg/webhook/mutatingwebhook/vm_mutate.go
+  - name: users.virtualmachines.webhook.sandbox
+    admissionReviewVersions:
+      - v1
+    clientConfig:
+      caBundle: ${CA_BUNDLE}
+      service:
+        name: member-operator-webhook
+        namespace: ${NAMESPACE}
+        path: "/mutate-virtual-machines"
+        port: 443
+    matchPolicy: Equivalent
+    rules:
+    - operations: ["CREATE"]
+      apiGroups: ["kubevirt.io"]
+      apiVersions: ["v1"]
+      resources: ["virtualmachines"]
+      scope: "Namespaced"
+    sideEffects: None
+    timeoutSeconds: 5
+    reinvocationPolicy: Never
+    failurePolicy: Fail
+    namespaceSelector:
+      matchLabels:
+        toolchain.dev.openshift.com/provider: codeready-toolchain
 - apiVersion: admissionregistration.k8s.io/v1
   kind: ValidatingWebhookConfiguration
   metadata:
@@ -189,7 +224,7 @@ objects:
       sideEffects: None
       timeoutSeconds: 5
       reinvocationPolicy: Never
-      failurePolicy: Ignore
+      failurePolicy: Fail
       namespaceSelector:
         matchLabels:
           toolchain.dev.openshift.com/provider: codeready-toolchain

pkg/webhook/deploy/deployment_test.go

@@ -223,19 +223,19 @@ func deployment(namespace, sa string, image string) string {
 }
 
 func mutatingWebhookConfig(namespace, caBundle string) string {
-	return fmt.Sprintf(`{"apiVersion":"admissionregistration.k8s.io/v1","kind":"MutatingWebhookConfiguration","metadata":{"name":"member-operator-webhook","labels":{"app":"member-operator-webhook","toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"webhooks":[{"name":"users.pods.webhook.sandbox","admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%s","service":{"name":"member-operator-webhook","namespace":"%s","path":"/mutate-users-pods","port":443}},"matchPolicy":"Equivalent","rules":[{"operations":["CREATE"],"apiGroups":[""],"apiVersions":["v1"],"resources":["pods"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5,"reinvocationPolicy":"Never","failurePolicy":"Ignore","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}}}]}`, caBundle, namespace)
+	return fmt.Sprintf(`{"apiVersion":"admissionregistration.k8s.io/v1","kind":"MutatingWebhookConfiguration","metadata":{"name":"member-operator-webhook","labels":{"app":"member-operator-webhook","toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"webhooks":[{"name":"users.pods.webhook.sandbox","admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/mutate-users-pods","port":443}},"matchPolicy":"Equivalent","rules":[{"operations":["CREATE"],"apiGroups":[""],"apiVersions":["v1"],"resources":["pods"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5,"reinvocationPolicy":"Never","failurePolicy":"Ignore","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}}},{"name":"users.virtualmachines.webhook.sandbox","admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/mutate-virtual-machines","port":443}},"matchPolicy":"Equivalent","rules":[{"operations":["CREATE"],"apiGroups":["kubevirt.io"],"apiVersions":["v1"],"resources":["virtualmachines"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5,"reinvocationPolicy":"Never","failurePolicy":"Fail","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}}}]}`, caBundle, namespace)
 }
 
 func validatingWebhookConfig(namespace, caBundle string) string {
-	return fmt.Sprintf(`{"apiVersion":"admissionregistration.k8s.io/v1","kind":"ValidatingWebhookConfiguration","metadata":{"labels":{"app":"member-operator-webhook","toolchain.dev.openshift.com/provider":"codeready-toolchain"},"name":"member-operator-validating-webhook"},"webhooks":[{"admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/validate-users-rolebindings","port":443}},"failurePolicy":"Ignore","matchPolicy":"Equivalent","name":"users.rolebindings.webhook.sandbox","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"reinvocationPolicy":"Never","rules":[{"apiGroups":["rbac.authorization.k8s.io","authorization.openshift.io"],"apiVersions":["v1"],"operations":["CREATE","UPDATE"],"resources":["rolebindings"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5},{"admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/validate-users-checlusters","port":443}},"failurePolicy":"Ignore","matchPolicy":"Equivalent","name":"users.checlusters.webhook.sandbox","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"reinvocationPolicy":"Never","rules":[{"apiGroups":["org.eclipse.che"],"apiVersions":["v2"],"operations":["CREATE"],"resources":["checlusters"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5},{"admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/validate-spacebindingrequests","port":443}},"failurePolicy":"Fail","matchPolicy":"Equivalent","name":"users.spacebindingrequests.webhook.sandbox","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"reinvocationPolicy":"Never","rules":[{"apiGroups":["toolchain.dev.openshift.com"],"apiVersions":["v1alpha1"],"operations":["CREATE", "UPDATE"],"resources":["spacebindingrequests"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5}]}`, caBundle, namespace)
+	return fmt.Sprintf(`{"apiVersion":"admissionregistration.k8s.io/v1","kind":"ValidatingWebhookConfiguration","metadata":{"labels":{"app":"member-operator-webhook","toolchain.dev.openshift.com/provider":"codeready-toolchain"},"name":"member-operator-validating-webhook"},"webhooks":[{"admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/validate-users-rolebindings","port":443}},"failurePolicy":"Ignore","matchPolicy":"Equivalent","name":"users.rolebindings.webhook.sandbox","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"reinvocationPolicy":"Never","rules":[{"apiGroups":["rbac.authorization.k8s.io","authorization.openshift.io"],"apiVersions":["v1"],"operations":["CREATE","UPDATE"],"resources":["rolebindings"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5},{"admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/validate-users-checlusters","port":443}},"failurePolicy":"Fail","matchPolicy":"Equivalent","name":"users.checlusters.webhook.sandbox","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"reinvocationPolicy":"Never","rules":[{"apiGroups":["org.eclipse.che"],"apiVersions":["v2"],"operations":["CREATE"],"resources":["checlusters"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5},{"admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/validate-spacebindingrequests","port":443}},"failurePolicy":"Fail","matchPolicy":"Equivalent","name":"users.spacebindingrequests.webhook.sandbox","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"reinvocationPolicy":"Never","rules":[{"apiGroups":["toolchain.dev.openshift.com"],"apiVersions":["v1alpha1"],"operations":["CREATE", "UPDATE"],"resources":["spacebindingrequests"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5}]}`, caBundle, namespace)
 }
 
 func serviceAccount(namespace string) string {
 	return fmt.Sprintf(`{"apiVersion": "v1","kind": "ServiceAccount", "metadata":{"name": "member-operator-webhook-sa", "namespace": "%s"}}`, namespace)
 }
 
 func clusterRole() string {
-	return `{"apiVersion": "rbac.authorization.k8s.io/v1","kind": "ClusterRole","metadata": {"creationTimestamp": null,"name": "webhook-role"}, "rules": [{"apiGroups": ["user.openshift.io"],"resources": ["identities","useridentitymappings","users"],"verbs": ["get","list","watch"]},{"apiGroups": ["toolchain.dev.openshift.com"],"resources": ["spacebindingrequests"],"verbs": ["get","list","watch"]}]}`
+	return `{"apiVersion": "rbac.authorization.k8s.io/v1","kind": "ClusterRole","metadata": {"creationTimestamp": null,"name": "webhook-role"}, "rules": [{"apiGroups": ["user.openshift.io"],"resources": ["identities","useridentitymappings","users"],"verbs": ["get","list","watch"]},{"apiGroups": ["toolchain.dev.openshift.com"],"resources": ["spacebindingrequests"],"verbs": ["get","list","watch"]},{"apiGroups": ["kubevirt.io"],"resources": ["virtualmachines"],"verbs": ["get","list","watch"]}]}`
 }
 
 func clusterRoleBinding(namespace string) string {

pkg/webhook/mutatingwebhook/mutate.go

@@ -5,87 +5,59 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"os"
 
-	"github.com/pkg/errors"
+	"github.com/go-logr/logr"
 	v1 "k8s.io/api/admission/v1"
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 var (
 	runtimeScheme = runtime.NewScheme()
 	codecs        = serializer.NewCodecFactory(runtimeScheme)
 	deserializer  = codecs.UniversalDeserializer()
-
-	log = logf.Log.WithName("users_pods_mutating_webhook")
-
-	patchContent = patchedContent()
-)
-
-const (
-	priority          = int32(-3)
-	priorityClassName = "sandbox-users-pods"
 )
 
-func patchedContent() []byte {
-	patchItems := []map[string]interface{}{
-		{
-			"op":    "replace",
-			"path":  "/spec/priorityClassName",
-			"value": priorityClassName,
-		},
-		{
-			"op":    "replace",
-			"path":  "/spec/priority",
-			"value": priority,
-		},
-	}
-
-	patchContent, err := json.Marshal(patchItems)
-	if err != nil {
-		log.Error(err, "unable marshal patch items")
-		os.Exit(1)
-	}
-	return patchContent
-}
+type mutateHandler func(admReview v1.AdmissionReview) *v1.AdmissionResponse
 
-func HandleMutate(w http.ResponseWriter, r *http.Request) {
+func handleMutate(logger logr.Logger, w http.ResponseWriter, r *http.Request, mutator mutateHandler) {
 	var respBody []byte
 	body, err := io.ReadAll(r.Body)
 	defer func() {
 		if err := r.Body.Close(); err != nil {
-			log.Error(err, "unable to close the body")
+			logger.Error(err, "unable to close the body")
 		}
 	}()
 	if err != nil {
-		log.Error(err, "unable to read the body of the request")
+		logger.Error(err, "unable to read the body of the request")
 		w.WriteHeader(http.StatusInternalServerError)
 		respBody = []byte("unable to read the body of the request")
 	} else {
 		// mutate the request
-		respBody = mutate(body)
+		respBody = mutate(logger, body, mutator)
 		w.WriteHeader(http.StatusOK)
 	}
 	if _, err := io.WriteString(w, string(respBody)); err != nil {
-		log.Error(err, "unable to write response")
+		logger.Error(err, "unable to write response")
 	}
 }
 
-func mutate(body []byte) []byte {
+func mutate(logger logr.Logger, body []byte, mutator mutateHandler) []byte {
 	admReview := v1.AdmissionReview{}
 	if _, _, err := deserializer.Decode(body, nil, &admReview); err != nil {
-		log.Error(err, "unable to deserialize the admission review object", "body", string(body))
+		logger.Error(err, "unable to deserialize the admission review object", "body", string(body))
+		admReview.Response = responseWithError(err)
+	} else if admReview.Request == nil {
+		err := fmt.Errorf("admission review request is nil")
+		logger.Error(err, "cannot read the admission review request", "AdmissionReview", admReview)
 		admReview.Response = responseWithError(err)
 	} else {
-		admReview.Response = createAdmissionReviewResponse(admReview)
+		admReview.Response = mutator(admReview)
 	}
 	responseBody, err := json.Marshal(admReview)
 	if err != nil {
-		log.Error(err, "unable to marshal the admission review with response", "admissionReview", admReview)
+		logger.Error(err, "unable to marshal the admission review with response", "admissionReview", admReview)
 	}
 	return responseBody
 }
@@ -97,34 +69,3 @@ func responseWithError(err error) *v1.AdmissionResponse {
 		},
 	}
 }
-
-func createAdmissionReviewResponse(admReview v1.AdmissionReview) *v1.AdmissionResponse {
-	if admReview.Request == nil {
-		err := fmt.Errorf("admission review request is nil")
-		log.Error(err, "cannot read the admission review request", "AdmissionReview", admReview)
-		return responseWithError(err)
-	}
-
-	// let's unmarshal the object to be sure that it's a pod
-	var pod *corev1.Pod
-	if err := json.Unmarshal(admReview.Request.Object.Raw, &pod); err != nil {
-		log.Error(err, "unable unmarshal pod json object", "AdmissionReview", admReview)
-		return responseWithError(errors.Wrapf(err, "unable unmarshal pod json object - raw request object: %v", admReview.Request.Object.Raw))
-	}
-
-	patchType := v1.PatchTypeJSONPatch
-	resp := &v1.AdmissionResponse{
-		Allowed:   true,
-		UID:       admReview.Request.UID,
-		PatchType: &patchType,
-	}
-	resp.AuditAnnotations = map[string]string{
-		"users_pods_mutating_webhook": "the sandbox-users-pods PriorityClass was set",
-	}
-
-	// instead of changing the pod object we need to tell K8s how to change the object
-	resp.Patch = patchContent
-
-	log.Info("the sandbox-users-pods PriorityClass was set to the pod", "pod-name", pod.Name, "namespace", pod.Namespace)
-	return resp
-}