JobSink: add webhook validation for spec.job (#7962)

* JobSink: add webhook validation for spec.job Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Randomize temporary Job name Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> --------- Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
knative · Jun 11, 2024 · b18b1b1 · b18b1b1
1 parent 0bce743
commit b18b1b1
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 1 deletion.
diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go
@@ -23,10 +23,12 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
+	kubeclient "knative.dev/pkg/client/injection/kube/client"
 	configmapinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/filtered"
 
 	eventingv1beta3 "knative.dev/eventing/pkg/apis/eventing/v1beta3"
 	"knative.dev/eventing/pkg/apis/feature"
+	"knative.dev/eventing/pkg/apis/sinks"
 	sinksv1alpha1 "knative.dev/eventing/pkg/apis/sinks/v1alpha1"
 	"knative.dev/eventing/pkg/auth"
 	"knative.dev/eventing/pkg/eventingtls"
@@ -156,9 +158,17 @@ func NewValidationAdmissionController(ctx context.Context, cmw configmap.Watcher
 	featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store"))
 	featureStore.WatchConfigs(cmw)
 
+	k8s := kubeclient.Get(ctx)
+
 	// Decorate contexts with the current state of the config.
 	ctxFunc := func(ctx context.Context) context.Context {
-		return featureStore.ToContext(channelStore.ToContext(pingstore.ToContext(store.ToContext(ctx))))
+		return sinks.WithConfig(
+			featureStore.ToContext(
+				channelStore.ToContext(
+					pingstore.ToContext(store.ToContext(ctx)))),
+			&sinks.Config{
+				KubeClient: k8s,
+			})
 	}
 
 	return validation.NewAdmissionController(ctx,

diff --git a/config/core/roles/webhook-clusterrole.yaml b/config/core/roles/webhook-clusterrole.yaml
@@ -175,3 +175,7 @@ rules:
   - apiGroups: ["apiextensions.k8s.io"]
     resources: ["customresourcedefinitions"]
     verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
+
+  - apiGroups: ["batch"]
+    resources: ["jobs"]
+    verbs: ["create"]
diff --git a/docs/sink/jobsink-invalid.yaml b/docs/sink/jobsink-invalid.yaml
@@ -0,0 +1,21 @@
+apiVersion: sinks.knative.dev/v1alpha1
+kind: JobSink
+metadata:
+  name: job-sink-invalid
+spec:
+  job:
+    apiVersion: batch/v1
+    kind: Job
+    spec:
+      completions: 12
+      parallelism: 3
+      template:
+        spec:
+          # restartPolicy: Never -> missing field
+          containers:
+            - name: main
+              image: docker.io/library/bash:5
+              command: [ "bash" ]        # example command simulating a bug which triggers the FailJob action
+              args:
+                - -c
+                - echo "Hello world!" && sleep 5
diff --git a/pkg/apis/sinks/register.go b/pkg/apis/sinks/register.go
@@ -17,7 +17,10 @@ limitations under the License.
 package sinks
 
 import (
+	"context"
+
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/kubernetes"
 )
 
 const (
@@ -31,3 +34,21 @@ var (
 		Resource: "jobsinks",
 	}
 )
+
+type Config struct {
+	KubeClient kubernetes.Interface
+}
+
+type configKey struct{}
+
+func WithConfig(ctx context.Context, cfg *Config) context.Context {
+	return context.WithValue(ctx, configKey{}, cfg)
+}
+
+func GetConfig(ctx context.Context) *Config {
+	v := ctx.Value(configKey{})
+	if v == nil {
+		panic("Missing value for config")
+	}
+	return v.(*Config)
+}
diff --git a/pkg/apis/sinks/v1alpha1/job_sink_validation.go b/pkg/apis/sinks/v1alpha1/job_sink_validation.go
@@ -19,10 +19,15 @@ package v1alpha1
 import (
 	"context"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apiserver/pkg/storage/names"
 	"knative.dev/pkg/apis"
+
+	"knative.dev/eventing/pkg/apis/sinks"
 )
 
 func (sink *JobSink) Validate(ctx context.Context) *apis.FieldError {
+	ctx = apis.WithinParent(ctx, sink.ObjectMeta)
 	return sink.Spec.Validate(ctx).ViaField("spec")
 }
 
@@ -33,5 +38,20 @@ func (sink *JobSinkSpec) Validate(ctx context.Context) *apis.FieldError {
 		return errs.Also(apis.ErrMissingOneOf("job"))
 	}
 
+	if sink.Job != nil {
+		job := sink.Job.DeepCopy()
+		job.Name = names.SimpleNameGenerator.GenerateName(apis.ParentMeta(ctx).Name)
+		_, err := sinks.GetConfig(ctx).KubeClient.
+			BatchV1().
+			Jobs(apis.ParentMeta(ctx).Namespace).
+			Create(ctx, job, metav1.CreateOptions{
+				DryRun:          []string{metav1.DryRunAll},
+				FieldValidation: metav1.FieldValidationStrict,
+			})
+		if err != nil {
+			return apis.ErrGeneric(err.Error(), "job")
+		}
+	}
+
 	return errs
 }