From b18b1b1f3ffff14ac5b416541107bc9c8f1daf3c Mon Sep 17 00:00:00 2001 From: Pierangelo Di Pilato Date: Tue, 11 Jun 2024 20:50:06 +0200 Subject: [PATCH] JobSink: add webhook validation for spec.job (#7962) * JobSink: add webhook validation for spec.job Signed-off-by: Pierangelo Di Pilato * Randomize temporary Job name Signed-off-by: Pierangelo Di Pilato --------- Signed-off-by: Pierangelo Di Pilato --- cmd/webhook/main.go | 12 ++++++++++- config/core/roles/webhook-clusterrole.yaml | 4 ++++ docs/sink/jobsink-invalid.yaml | 21 +++++++++++++++++++ pkg/apis/sinks/register.go | 21 +++++++++++++++++++ .../sinks/v1alpha1/job_sink_validation.go | 20 ++++++++++++++++++ 5 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 docs/sink/jobsink-invalid.yaml diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index 9dd361d90ef..1c9c25fa761 100644 --- 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 index ae66a35c05a..9fdeb70a6a9 100644 --- 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 new file mode 100644 index 00000000000..997c466b1ac --- /dev/null +++ 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 index b37303c509c..676fa75e841 100644 --- 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 index 0fe178dc65b..7ed631ba3c3 100644 --- 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 }