feat: runtimeclass default support (#1165)

* fix(makefile): no need specifying ginkgo version Signed-off-by: Dario Tranchitella <[email protected]> * fix(chore): referring to make using shortcut Signed-off-by: Dario Tranchitella <[email protected]> * feat: default runtimeclass webhook Signed-off-by: Dario Tranchitella <[email protected]> * feat(helm): default runtimeclass spec Signed-off-by: Dario Tranchitella <[email protected]> --------- Signed-off-by: Dario Tranchitella <[email protected]>
projectcapsule · Aug 20, 2024 · a49c57b · a49c57b
1 parent d620b04
commit a49c57b
Show file tree

Hide file tree

Showing 9 changed files with 135 additions and 53 deletions.
diff --git a/Makefile b/Makefile
@@ -199,9 +199,8 @@ controller-gen: ## Download controller-gen locally if necessary.
 	$(call go-install-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_GEN_VERSION))
 
 GINKGO         := $(shell pwd)/bin/ginkgo
-GINGKO_VERSION := v2.17.2
 ginkgo: ## Download ginkgo locally if necessary.
-	$(call go-install-tool,$(GINKGO),github.com/onsi/ginkgo/v2/ginkgo@$(GINGKO_VERSION))
+	$(call go-install-tool,$(GINKGO),github.com/onsi/ginkgo/v2/ginkgo)
 
 CT         := $(shell pwd)/bin/ct
 CT_VERSION := v3.10.1
@@ -277,7 +276,7 @@ e2e/%: ginkgo
 
 e2e-build/%:
 	kind create cluster --wait=60s --name capsule --image=kindest/node:$*
-	make e2e-install
+	$(MAKE) e2e-install
 
 .PHONY: e2e-install
 e2e-install: e2e-load-image

diff --git a/api/v1beta2/tenant_types.go b/api/v1beta2/tenant_types.go
@@ -43,7 +43,7 @@ type TenantSpec struct {
 	// Specifies the allowed RuntimeClasses assigned to the Tenant.
 	// Capsule assures that all Pods resources created in the Tenant can use only one of the allowed RuntimeClasses.
 	// Optional.
-	RuntimeClasses *api.SelectorAllowedListSpec `json:"runtimeClasses,omitempty"`
+	RuntimeClasses *api.DefaultAllowedListSpec `json:"runtimeClasses,omitempty"`
 	// Specifies the allowed priorityClasses assigned to the Tenant.
 	// Capsule assures that all Pods resources created in the Tenant can use only one of the allowed PriorityClasses.
 	// A default value can be specified, and all the Pod resources created will inherit the declared class.

diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go
diff --git a/charts/capsule/crds/capsule.clastix.io_tenants.yaml b/charts/capsule/crds/capsule.clastix.io_tenants.yaml
@@ -2139,6 +2139,8 @@ spec:
                     type: array
                   allowedRegex:
                     type: string
+                  default:
+                    type: string
                   matchExpressions:
                     description: matchExpressions is a list of label selector requirements.
                       The requirements are ANDed.

diff --git a/e2e/pod_runtime_class_test.go b/e2e/pod_runtime_class_test.go
@@ -33,14 +33,17 @@ var _ = Describe("enforcing a Runtime Class", func() {
 					Kind: "User",
 				},
 			},
-			RuntimeClasses: &api.SelectorAllowedListSpec{
-				AllowedListSpec: api.AllowedListSpec{
-					Exact: []string{"legacy"},
-					Regex: "^hardened-.*$",
-				},
-				LabelSelector: metav1.LabelSelector{
-					MatchLabels: map[string]string{
-						"env": "customers",
+			RuntimeClasses: &api.DefaultAllowedListSpec{
+				Default: "default-runtime",
+				SelectorAllowedListSpec: api.SelectorAllowedListSpec{
+					AllowedListSpec: api.AllowedListSpec{
+						Exact: []string{"legacy"},
+						Regex: "^hardened-.*$",
+					},
+					LabelSelector: metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"env": "customers",
+						},
 					},
 				},
 			},
@@ -221,4 +224,49 @@ var _ = Describe("enforcing a Runtime Class", func() {
 		}
 	})
 
+	It("should auto assign the default", func() {
+		ns := NewNamespace("rc-default")
+
+		NamespaceCreation(ns, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
+
+		runtime := &nodev1.RuntimeClass{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "default-runtime",
+			},
+			Handler: "custom-handler",
+		}
+		Expect(k8sClient.Create(context.TODO(), runtime)).Should(Succeed())
+		defer func() {
+			Expect(k8sClient.Delete(context.TODO(), runtime)).Should(Succeed())
+		}()
+
+		pod := corev1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "rc-default",
+				Namespace: ns.Name,
+			},
+			Spec: corev1.PodSpec{
+				Containers: []corev1.Container{
+					{
+						Name:  "container",
+						Image: "quay.io/google-containers/pause-amd64:3.0",
+					},
+				},
+			},
+		}
+
+		cs := ownerClient(tnt.Spec.Owners[0])
+
+		var createdPod *corev1.Pod
+
+		EventuallyCreation(func() (err error) {
+			createdPod, err = cs.CoreV1().Pods(ns.GetName()).Create(context.Background(), &pod, metav1.CreateOptions{})
+
+			return err
+		}).Should(Succeed())
+
+		Expect(createdPod.Spec.RuntimeClassName).NotTo(BeNil())
+		_, err := Equal(createdPod.Spec.RuntimeClassName).Match(tnt.Spec.RuntimeClasses.Default)
+		Expect(err).NotTo(HaveOccurred())
+	})
 })
diff --git a/pkg/webhook/defaults/pods.go b/pkg/webhook/defaults/pods.go
@@ -11,79 +11,116 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	schedulev1 "k8s.io/api/scheduling/v1"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
-	capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2"
+	"github.com/projectcapsule/capsule/pkg/api"
 	"github.com/projectcapsule/capsule/pkg/webhook/utils"
 )
 
 func mutatePodDefaults(ctx context.Context, req admission.Request, c client.Client, decoder admission.Decoder, recorder record.EventRecorder, namespace string) *admission.Response {
-	var err error
-
-	pod := &corev1.Pod{}
-	if err = decoder.Decode(req, pod); err != nil {
+	var pod corev1.Pod
+	if err := decoder.Decode(req, &pod); err != nil {
 		return utils.ErroredResponse(err)
 	}
 
 	pod.SetNamespace(namespace)
 
-	var tnt *capsulev1beta2.Tenant
+	tnt, tErr := utils.TenantByStatusNamespace(ctx, c, pod.Namespace)
+	if tErr != nil {
+		return utils.ErroredResponse(tErr)
+	} else if tnt == nil {
+		return nil
+	}
+
+	var err error
 
-	tnt, err = utils.TenantByStatusNamespace(ctx, c, pod.Namespace)
-	if err != nil {
-		return utils.ErroredResponse(err)
+	pcMutated, pcErr := handlePriorityClassDefault(ctx, c, tnt.Spec.PriorityClasses, &pod)
+	if pcErr != nil {
+		return utils.ErroredResponse(pcErr)
+	} else if pcMutated {
+		defer func() {
+			if err == nil {
+				recorder.Eventf(tnt, corev1.EventTypeNormal, "TenantDefault", "Assigned Tenant default Priority Class %s to %s/%s", tnt.Spec.PriorityClasses.Default, pod.Namespace, pod.Name)
+			}
+		}()
 	}
 
-	if tnt == nil {
+	rcMutated := handleRuntimeClassDefault(tnt.Spec.RuntimeClasses, &pod)
+	if rcMutated {
+		defer func() {
+			if err == nil {
+				recorder.Eventf(tnt, corev1.EventTypeNormal, "TenantDefault", "Assigned Tenant default Runtime Class %s to %s/%s", tnt.Spec.RuntimeClasses.Default, pod.Namespace, pod.Name)
+			}
+		}()
+	}
+
+	if !rcMutated && !pcMutated {
 		return nil
 	}
 
-	allowed := tnt.Spec.PriorityClasses
+	var marshaled []byte
+
+	if marshaled, err = json.Marshal(pod); err != nil {
+		return utils.ErroredResponse(err)
+	}
+
+	return ptr.To(admission.PatchResponseFromRaw(req.Object.Raw, marshaled))
+}
 
+func handleRuntimeClassDefault(allowed *api.DefaultAllowedListSpec, pod *corev1.Pod) (mutated bool) {
 	if allowed == nil || allowed.Default == "" {
-		return nil
+		return false
 	}
 
-	priorityClassPod := pod.Spec.PriorityClassName
+	runtimeClass := pod.Spec.RuntimeClassName
+
+	switch {
+	case allowed.Default == "":
+		return false
+	case runtimeClass != nil && *runtimeClass != "":
+		return false
+	case runtimeClass != nil && *runtimeClass != allowed.Default:
+		return false
+	default:
+		pod.Spec.RuntimeClassName = &allowed.Default
+
+		return true
+	}
+}
+
+func handlePriorityClassDefault(ctx context.Context, c client.Client, allowed *api.DefaultAllowedListSpec, pod *corev1.Pod) (mutated bool, err error) {
+	if allowed == nil || allowed.Default == "" {
+		return false, nil
+	}
 
-	var mutate bool
+	priorityClassPod := pod.Spec.PriorityClassName
 
 	var cpc *schedulev1.PriorityClass
 	// PriorityClass name is empty, if no GlobalDefault is set and no PriorityClass was given on pod
 	if len(priorityClassPod) > 0 && priorityClassPod != allowed.Default {
 		cpc, err = utils.GetPriorityClassByName(ctx, c, priorityClassPod)
 		// Should not happen, since API already checks if PC present
 		if err != nil {
-			response := admission.Denied(NewPriorityClassError(priorityClassPod, err).Error())
-
-			return &response
+			return false, NewPriorityClassError(priorityClassPod, err)
 		}
 	} else {
-		mutate = true
+		mutated = true
 	}
 
-	if mutate = mutate || (utils.IsDefaultPriorityClass(cpc) && cpc.GetName() != allowed.Default); !mutate {
-		return nil
+	if mutated = mutated || (utils.IsDefaultPriorityClass(cpc) && cpc.GetName() != allowed.Default); !mutated {
+		return false, nil
 	}
 
 	pc, err := utils.GetPriorityClassByName(ctx, c, allowed.Default)
 	if err != nil {
-		return utils.ErroredResponse(fmt.Errorf("failed to assign tenant default Priority Class: %w", err))
+		return false, fmt.Errorf("failed to assign tenant default Priority Class: %w", err)
 	}
 
 	pod.Spec.PreemptionPolicy = pc.PreemptionPolicy
 	pod.Spec.Priority = &pc.Value
 	pod.Spec.PriorityClassName = pc.Name
-	// Marshal Pod
-	marshaled, err := json.Marshal(pod)
-	if err != nil {
-		return utils.ErroredResponse(err)
-	}
-
-	recorder.Eventf(tnt, corev1.EventTypeNormal, "TenantDefault", "Assigned Tenant default Priority Class %s to %s/%s", allowed.Default, pod.Namespace, pod.Name)
-
-	response := admission.PatchResponseFromRaw(req.Object.Raw, marshaled)
 
-	return &response
+	return true, nil
 }
diff --git a/pkg/webhook/pod/runtimeclass.go b/pkg/webhook/pod/runtimeclass.go
@@ -88,8 +88,8 @@ func (h *runtimeClass) validate(ctx context.Context, c client.Client, decoder ad
 	case allowed == nil:
 		// Enforcement is not in place, skipping it at all
 		return nil
-	case len(runtimeClassName) == 0:
-		// We don't have to force Pod to specify a RuntimeClass
+	case len(runtimeClassName) == 0 || runtimeClassName == allowed.Default:
+		// Delegating mutating webhook to specify a default RuntimeClass
 		return nil
 	case !allowed.MatchSelectByName(class):
 		recorder.Eventf(tnt, corev1.EventTypeWarning, "ForbiddenRuntimeClass", "Pod %s/%s is using Runtime Class %s is forbidden for the current Tenant", pod.Namespace, pod.Name, runtimeClassName)

diff --git a/pkg/webhook/pod/runtimeclass_errors.go b/pkg/webhook/pod/runtimeclass_errors.go
@@ -12,10 +12,10 @@ import (
 
 type podRuntimeClassForbiddenError struct {
 	runtimeClassName string
-	spec             api.SelectorAllowedListSpec
+	spec             api.DefaultAllowedListSpec
 }
 
-func NewPodRuntimeClassForbidden(runtimeClassName string, spec api.SelectorAllowedListSpec) error {
+func NewPodRuntimeClassForbidden(runtimeClassName string, spec api.DefaultAllowedListSpec) error {
 	return &podRuntimeClassForbiddenError{
 		runtimeClassName: runtimeClassName,
 		spec:             spec,
@@ -25,5 +25,5 @@ func NewPodRuntimeClassForbidden(runtimeClassName string, spec api.SelectorAllow
 func (f podRuntimeClassForbiddenError) Error() (err string) {
 	err = fmt.Sprintf("Pod Runtime Class %s is forbidden for the current Tenant: ", f.runtimeClassName)
 
-	return utils.AllowedValuesErrorMessage(f.spec, err)
+	return utils.DefaultAllowedValuesErrorMessage(f.spec, err)
 }
diff --git a/pkg/webhook/utils/error.go b/pkg/webhook/utils/error.go
@@ -20,10 +20,6 @@ func ErroredResponse(err error) *admission.Response {
 }
 
 func DefaultAllowedValuesErrorMessage(allowed api.DefaultAllowedListSpec, err string) string {
-	return AllowedValuesErrorMessage(allowed.SelectorAllowedListSpec, err)
-}
-
-func AllowedValuesErrorMessage(allowed api.SelectorAllowedListSpec, err string) string {
 	var extra []string
 	if len(allowed.Exact) > 0 {
 		extra = append(extra, fmt.Sprintf("use one from the following list (%s)", strings.Join(allowed.Exact, ", ")))