From a49c57bb5bbd04abd8b6de712a071b933621a64b Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Tue, 20 Aug 2024 10:50:26 +0200 Subject: [PATCH] feat: runtimeclass default support (#1165) * fix(makefile): no need specifying ginkgo version Signed-off-by: Dario Tranchitella * fix(chore): referring to make using shortcut Signed-off-by: Dario Tranchitella * feat: default runtimeclass webhook Signed-off-by: Dario Tranchitella * feat(helm): default runtimeclass spec Signed-off-by: Dario Tranchitella --------- Signed-off-by: Dario Tranchitella --- Makefile | 5 +- api/v1beta2/tenant_types.go | 2 +- api/v1beta2/zz_generated.deepcopy.go | 2 +- .../crds/capsule.clastix.io_tenants.yaml | 2 + e2e/pod_runtime_class_test.go | 64 ++++++++++-- pkg/webhook/defaults/pods.go | 99 +++++++++++++------ pkg/webhook/pod/runtimeclass.go | 4 +- pkg/webhook/pod/runtimeclass_errors.go | 6 +- pkg/webhook/utils/error.go | 4 - 9 files changed, 135 insertions(+), 53 deletions(-) diff --git a/Makefile b/Makefile index 45346b28..e2ee7ab7 100644 --- 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 index 52e15513..bb67c3bd 100644 --- 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 index b64cd872..f9feb05e 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -755,7 +755,7 @@ func (in *TenantSpec) DeepCopyInto(out *TenantSpec) { } if in.RuntimeClasses != nil { in, out := &in.RuntimeClasses, &out.RuntimeClasses - *out = new(api.SelectorAllowedListSpec) + *out = new(api.DefaultAllowedListSpec) (*in).DeepCopyInto(*out) } if in.PriorityClasses != nil { diff --git a/charts/capsule/crds/capsule.clastix.io_tenants.yaml b/charts/capsule/crds/capsule.clastix.io_tenants.yaml index c073502f..7441080d 100644 --- 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 index 259b35bc..d3c3096f 100644 --- 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 index bd21f51a..2ca8dcde 100644 --- a/pkg/webhook/defaults/pods.go +++ b/pkg/webhook/defaults/pods.go @@ -11,43 +11,91 @@ 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 @@ -55,35 +103,24 @@ func mutatePodDefaults(ctx context.Context, req admission.Request, c client.Clie 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 index 37b9cab1..5bbd976a 100644 --- 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 index 1dce1999..cae23663 100644 --- 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 index a24036ff..50998398 100644 --- 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, ", ")))