diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 98fb9aa9e8..5ff7051259 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -727,3 +727,24 @@ webhooks: resources: - uniteddeployments sideEffects: None +- admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-apps-kruise-io-v1alpha1-ephemeraljob + failurePolicy: Fail + name: vephemeraljob.kb.io + rules: + - apiGroups: + - apps.kruise.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - ephemeraljobs + sideEffects: None diff --git a/pkg/controller/ephemeraljob/ephemeraljob_controller.go b/pkg/controller/ephemeraljob/ephemeraljob_controller.go index 029f32fa6c..004ff7d2bc 100644 --- a/pkg/controller/ephemeraljob/ephemeraljob_controller.go +++ b/pkg/controller/ephemeraljob/ephemeraljob_controller.go @@ -34,6 +34,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "k8s.io/klog/v2" kubecontroller "k8s.io/kubernetes/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/client" @@ -65,8 +66,9 @@ func Add(mgr manager.Manager) error { // newReconciler returns a new reconcile.Reconciler func newReconciler(mgr manager.Manager) *ReconcileEphemeralJob { return &ReconcileEphemeralJob{ - Client: utilclient.NewClientFromManager(mgr, "ephemeraljob-controller"), - scheme: mgr.GetScheme(), + Client: utilclient.NewClientFromManager(mgr, "ephemeraljob-controller"), + scheme: mgr.GetScheme(), + eventRecorder: mgr.GetEventRecorderFor("ephemeraljob-controller"), } } @@ -98,7 +100,8 @@ var _ reconcile.Reconciler = &ReconcileEphemeralJob{} // ReconcileEphemeralJob reconciles a ImagePullJob object type ReconcileEphemeralJob struct { client.Client - scheme *runtime.Scheme + scheme *runtime.Scheme + eventRecorder record.EventRecorder } // +kubebuilder:rbac:groups=apps.kruise.io,resources=ephemeraljobs,verbs=get;list;watch;update;patch;delete @@ -345,12 +348,15 @@ func (r *ReconcileEphemeralJob) syncTargetPods(job *appsv1alpha1.EphemeralJob, t scaleExpectations.ExpectScale(key, expectations.Create, podEphemeralContainerName) } if err := control.CreateEphemeralContainer(pod); err != nil { + r.eventRecorder.Event(job, v1.EventTypeWarning, "CreateFailed", + fmt.Sprintf("Failed to create ephemeral container for pod %s: %v", pod.Name, err)) for _, podEphemeralContainerName := range getPodEphemeralContainers(pod, job) { scaleExpectations.ObserveScale(key, expectations.Create, podEphemeralContainerName) } return err } - + r.eventRecorder.Event(job, v1.EventTypeNormal, "CreateSuccessfully", + fmt.Sprintf("create ephemeral container for pod %s successfully", pod.Name)) return nil }) @@ -440,12 +446,14 @@ func (r *ReconcileEphemeralJob) removeEphemeralContainers(job *appsv1alpha1.Ephe return err } - var errors error + control := econtainer.New(job) for _, pod := range targetPods { - if e := econtainer.New(job).RemoveEphemeralContainer(pod); e != nil { - errors = e + if err = control.RemoveEphemeralContainer(pod); err != nil { + r.eventRecorder.Event(job, v1.EventTypeWarning, "RemoveFailed", + fmt.Sprintf("Failed to remove ephemeral container for pod %s: %v", pod.Name, err)) + return err } } - return errors + return nil } diff --git a/pkg/webhook/ephemeraljob/validating/ephemeraljob_validating_handler.go b/pkg/webhook/ephemeraljob/validating/ephemeraljob_validating_handler.go new file mode 100644 index 0000000000..7e32410b85 --- /dev/null +++ b/pkg/webhook/ephemeraljob/validating/ephemeraljob_validating_handler.go @@ -0,0 +1,100 @@ +/* +Copyright 2023 The Kruise Authors. + +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 validating + +import ( + "context" + "net/http" + "unsafe" + + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/klog/v2" + "k8s.io/kubernetes/pkg/apis/core" + corev1 "k8s.io/kubernetes/pkg/apis/core/v1" + "k8s.io/kubernetes/pkg/apis/core/validation" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// EphemeralJobCreateUpdateHandler handles ImagePullJob +type EphemeralJobCreateUpdateHandler struct { + // Decoder decodes objects + Decoder *admission.Decoder +} + +var _ admission.Handler = &EphemeralJobCreateUpdateHandler{} + +// Handle handles admission requests. +func (h *EphemeralJobCreateUpdateHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + job := &appsv1alpha1.EphemeralJob{} + + err := h.Decoder.Decode(req, job) + if err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + if err := validateEphemeralJobSpec(&job.Spec, field.NewPath("spec")); err != nil { + klog.Warningf("Error validate EphemeralJob %s/%s: %v", job.Namespace, job.Name, err) + return admission.Errored(http.StatusBadRequest, err.ToAggregate()) + } + + return admission.ValidationResponse(true, "allowed") +} + +func validateEphemeralJobSpec(spec *appsv1alpha1.EphemeralJobSpec, path *field.Path) field.ErrorList { + var allErrs field.ErrorList + if spec.Selector != nil { + if spec.Selector.MatchLabels != nil || spec.Selector.MatchExpressions != nil { + if _, err := metav1.LabelSelectorAsSelector(spec.Selector); err != nil { + allErrs = append(allErrs, field.Invalid(path.Child("selector"), spec.Selector, err.Error())) + } + } + } else { + allErrs = append(allErrs, field.Invalid(path.Child("selector"), spec.Selector, "selector must be not empty")) + } + + var ephemeralContainers []v1.EphemeralContainer + ecPath := path.Child("template").Child("ephemeralContainers") + for index, ec := range spec.Template.EphemeralContainers { + idxPath := ecPath.Index(index) + + // VolumeMount subpaths have the potential to leak resources since they're implemented with bind mounts + // that aren't cleaned up until the pod exits. Since they also imply that the container is being used + // as part of the workload, they're disallowed entirely. + for i, vm := range ec.VolumeMounts { + if vm.SubPath != "" { + allErrs = append(allErrs, field.Forbidden(idxPath.Child("volumeMounts").Index(i).Child("subPath"), "cannot be set for an Ephemeral Container")) + } + if vm.SubPathExpr != "" { + allErrs = append(allErrs, field.Forbidden(idxPath.Child("volumeMounts").Index(i).Child("subPathExpr"), "cannot be set for an Ephemeral Container")) + } + } + + // VolumeMount cannot be validated further by ValidatePodEphemeralContainersUpdate method because we do not know the volumes and target container of target Pods + ec.VolumeMounts, ec.TargetContainerName = nil, "" + corev1.SetDefaults_Container((*v1.Container)(&ec.EphemeralContainerCommon)) + ephemeralContainers = append(ephemeralContainers, ec) + } + + // validateEphemeralContainers is a private method in k8s validation package, so we have to use ValidatePodEphemeralContainersUpdate + // to validate fields of the fields of ephemeral containers + mockedNewPod := &core.Pod{Spec: core.PodSpec{EphemeralContainers: *(*[]core.EphemeralContainer)(unsafe.Pointer(&ephemeralContainers))}} + mockedOldPod := &core.Pod{Spec: core.PodSpec{EphemeralContainers: *(*[]core.EphemeralContainer)(unsafe.Pointer(&ephemeralContainers))}} + return append(allErrs, validation.ValidatePodEphemeralContainersUpdate(mockedNewPod, mockedOldPod, validation.PodValidationOptions{})...) +} diff --git a/pkg/webhook/ephemeraljob/validating/ephemeraljob_validating_handler_test.go b/pkg/webhook/ephemeraljob/validating/ephemeraljob_validating_handler_test.go new file mode 100644 index 0000000000..6aed4b847e --- /dev/null +++ b/pkg/webhook/ephemeraljob/validating/ephemeraljob_validating_handler_test.go @@ -0,0 +1,151 @@ +/* +Copyright 2023 The Kruise Authors. + +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 validating + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "testing" + + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +var ( + ecDemo = v1.EphemeralContainer{ + EphemeralContainerCommon: v1.EphemeralContainerCommon{ + Name: "ec-1", + Image: "busybox:1.32", + Command: []string{"/bin/sh"}, + Args: []string{"-c", "sleep 100d"}, + Env: []v1.EnvVar{ + {Name: "key_1", Value: "value_1"}, + {Name: "key_2", Value: "value_2"}, + }, + VolumeMounts: []v1.VolumeMount{ + { + Name: "vol-1", + MountPath: "/home/logs", + }, + }, + }, + TargetContainerName: "main", + } + + selector = metav1.LabelSelector{MatchLabels: map[string]string{"app": "demo"}} +) + +func TestValidateEphemeralJobSpec(t *testing.T) { + successfulCases := map[string]func() *appsv1alpha1.EphemeralJobSpec{ + "normal case": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + return &appsv1alpha1.EphemeralJobSpec{Selector: &selector, Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + "without comm": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + ec.Command = nil + return &appsv1alpha1.EphemeralJobSpec{Selector: &selector, Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + "without args": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + ec.Args = nil + return &appsv1alpha1.EphemeralJobSpec{Selector: &selector, Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + "without env": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + ec.Env = nil + return &appsv1alpha1.EphemeralJobSpec{Selector: &selector, Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + "without volumeMounts": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + ec.VolumeMounts = nil + return &appsv1alpha1.EphemeralJobSpec{Selector: &selector, Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + "without targetContainer": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + ec.TargetContainerName = "" + return &appsv1alpha1.EphemeralJobSpec{Selector: &selector, Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + } + + for name, cs := range successfulCases { + t.Run(name, func(t *testing.T) { + allErrs := validateEphemeralJobSpec(cs(), field.NewPath("spec")) + if len(allErrs) != 0 { + t.Fatalf("got unexpected error: %v", allErrs.ToAggregate()) + } + }) + } + + failedCases := map[string]func() *appsv1alpha1.EphemeralJobSpec{ + "without selector": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + return &appsv1alpha1.EphemeralJobSpec{Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + "without image": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + ec.Image = "" + return &appsv1alpha1.EphemeralJobSpec{Selector: &selector, Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + "without name": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + ec.Name = "" + return &appsv1alpha1.EphemeralJobSpec{Selector: &selector, Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + "with ports": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + ec.Ports = []v1.ContainerPort{{Name: "web", ContainerPort: 80, Protocol: v1.ProtocolTCP}} + return &appsv1alpha1.EphemeralJobSpec{Selector: &selector, Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + "with resources": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + ec.Resources = v1.ResourceRequirements{Limits: v1.ResourceList{"cpu": *resource.NewQuantity(100000, resource.DecimalSI)}} + return &appsv1alpha1.EphemeralJobSpec{Selector: &selector, Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + "with subPath": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + ec.VolumeMounts[0].SubPath = "sub_path" + return &appsv1alpha1.EphemeralJobSpec{Selector: &selector, Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + "with probe": func() *appsv1alpha1.EphemeralJobSpec { + ec := ecDemo.DeepCopy() + ec.ReadinessProbe = &v1.Probe{ + Handler: v1.Handler{ + Exec: &v1.ExecAction{ + Command: []string{"/bin/sh"}, + }, + }, + InitialDelaySeconds: 10, + FailureThreshold: 3, + SuccessThreshold: 1, + TimeoutSeconds: 10, + PeriodSeconds: 10, + } + return &appsv1alpha1.EphemeralJobSpec{Selector: &selector, Template: appsv1alpha1.EphemeralContainerTemplateSpec{EphemeralContainers: []v1.EphemeralContainer{*ec}}} + }, + } + + for name, cs := range failedCases { + t.Run(name, func(t *testing.T) { + allErrs := validateEphemeralJobSpec(cs(), field.NewPath("spec")) + if len(allErrs) != 1 { + t.Fatalf("got unexpected error: %v", allErrs.ToAggregate()) + } + }) + } +} diff --git a/pkg/webhook/ephemeraljob/validating/webhooks.go b/pkg/webhook/ephemeraljob/validating/webhooks.go new file mode 100644 index 0000000000..d0f96a9629 --- /dev/null +++ b/pkg/webhook/ephemeraljob/validating/webhooks.go @@ -0,0 +1,30 @@ +/* +Copyright 2023 The Kruise Authors. + +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 validating + +import ( + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// +kubebuilder:webhook:path=/validate-apps-kruise-io-v1alpha1-ephemeraljob,mutating=false,failurePolicy=fail,sideEffects=None,admissionReviewVersions=v1;v1beta1,groups=apps.kruise.io,resources=ephemeraljobs,verbs=create;update,versions=v1alpha1,name=vephemeraljob.kb.io + +var ( + // HandlerMap contains admission webhook handlers + HandlerMap = map[string]admission.Handler{ + "validate-apps-kruise-io-v1alpha1-ephemeraljob": &EphemeralJobCreateUpdateHandler{}, + } +)