Add validating webhook for ephemeral job

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

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
 }

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{})...)
+}

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())
+			}
+		})
+	}
+}

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{},
+	}
+)