diff --git a/go.mod b/go.mod index 610c88b8d4..46b32ee7a4 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( github.com/spf13/cobra v1.8.1 github.com/spf13/pflag v1.0.5 go.uber.org/zap v1.27.0 + gomodules.xyz/jsonpatch/v2 v2.4.0 k8s.io/api v0.31.1 k8s.io/apimachinery v0.31.1 k8s.io/apiserver v0.31.1 @@ -128,7 +129,6 @@ require ( golang.org/x/text v0.17.0 // indirect golang.org/x/time v0.5.0 // indirect golang.org/x/tools v0.24.0 // indirect - gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/genproto v0.0.0-20240528184218-531527333157 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 // indirect diff --git a/pkg/controller/jobframework/base_webhook.go b/pkg/controller/jobframework/base_webhook.go index 2239e1c938..fe5e9e8bb5 100644 --- a/pkg/controller/jobframework/base_webhook.go +++ b/pkg/controller/jobframework/base_webhook.go @@ -24,6 +24,8 @@ import ( "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "sigs.k8s.io/kueue/pkg/controller/jobframework/webhook" ) // BaseWebhook applies basic defaulting and validation for jobs. @@ -39,9 +41,9 @@ func DefaultWebhookFactory(job GenericJob, fromObject func(runtime.Object) Gener ManageJobsWithoutQueueName: options.ManageJobsWithoutQueueName, FromObject: fromObject, } - return ctrl.NewWebhookManagedBy(mgr). + return webhook.WebhookManagedBy(mgr). For(job.Object()). - WithDefaulter(wh). + WithMutationHandler(webhook.WithLosslessDefaulter(mgr.GetScheme(), job.Object(), wh)). WithValidator(wh). Complete() } diff --git a/pkg/controller/jobframework/noop_webhook.go b/pkg/controller/jobframework/noop_webhook.go index 889b50262f..f6353ca566 100644 --- a/pkg/controller/jobframework/noop_webhook.go +++ b/pkg/controller/jobframework/noop_webhook.go @@ -24,11 +24,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -type webhook struct { +type noopWebhook struct { } func setupNoopWebhook(mgr ctrl.Manager, apiType runtime.Object) error { - wh := &webhook{} + wh := &noopWebhook{} return ctrl.NewWebhookManagedBy(mgr). For(apiType). WithDefaulter(wh). @@ -37,21 +37,21 @@ func setupNoopWebhook(mgr ctrl.Manager, apiType runtime.Object) error { } // Default implements webhook.CustomDefaulter so a webhook will be registered for the type -func (w *webhook) Default(context.Context, runtime.Object) error { +func (w *noopWebhook) Default(context.Context, runtime.Object) error { return nil } // ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type -func (w *webhook) ValidateCreate(context.Context, runtime.Object) (admission.Warnings, error) { +func (w *noopWebhook) ValidateCreate(context.Context, runtime.Object) (admission.Warnings, error) { return nil, nil } // ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type -func (w *webhook) ValidateUpdate(context.Context, runtime.Object, runtime.Object) (admission.Warnings, error) { +func (w *noopWebhook) ValidateUpdate(context.Context, runtime.Object, runtime.Object) (admission.Warnings, error) { return nil, nil } // ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type -func (w *webhook) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) { +func (w *noopWebhook) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) { return nil, nil } diff --git a/pkg/controller/jobframework/webhook/builder.go b/pkg/controller/jobframework/webhook/builder.go new file mode 100644 index 0000000000..d6d551b4dc --- /dev/null +++ b/pkg/controller/jobframework/webhook/builder.go @@ -0,0 +1,263 @@ +/* +Copyright 2024 The Kubernetes 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 webhook + +import ( + "errors" + "net/http" + "net/url" + "strings" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/rest" + "k8s.io/klog/v2" + + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "sigs.k8s.io/controller-runtime/pkg/webhook/conversion" +) + +// This code is copied from https://github.com/kubernetes-sigs/controller-runtime/blob/896f6ded750155f9ecfdf4d8e10a26fc3fb78384/pkg/builder/webhook.go +// with one modification to get full control of the construction of patches: +// replacing CustomDefaulter with an admission.Handler. +// TODO(#3137): remove this file + +// WebhookBuilder builds a Webhook. +type WebhookBuilder struct { + apiType runtime.Object + mutationHandler admission.Handler + customValidator admission.CustomValidator + gvk schema.GroupVersionKind + mgr manager.Manager + config *rest.Config + recoverPanic *bool + logConstructor func(base logr.Logger, req *admission.Request) logr.Logger + err error +} + +// WebhookManagedBy returns a new webhook builder. +func WebhookManagedBy(m manager.Manager) *WebhookBuilder { + return &WebhookBuilder{mgr: m} +} + +// TODO(droot): update the GoDoc for conversion. + +// For takes a runtime.Object which should be a CR. +// If the given object implements the admission.Defaulter interface, a MutatingWebhook will be wired for this type. +// If the given object implements the admission.Validator interface, a ValidatingWebhook will be wired for this type. +func (blder *WebhookBuilder) For(apiType runtime.Object) *WebhookBuilder { + if blder.apiType != nil { + blder.err = errors.New("For(...) should only be called once, could not assign multiple objects for webhook registration") + } + blder.apiType = apiType + return blder +} + +// WithDefaulter takes an admission.CustomDefaulter interface, a MutatingWebhook will be wired for this type. +func (blder *WebhookBuilder) WithMutationHandler(h admission.Handler) *WebhookBuilder { + blder.mutationHandler = h + return blder +} + +// WithValidator takes a admission.CustomValidator interface, a ValidatingWebhook will be wired for this type. +func (blder *WebhookBuilder) WithValidator(validator admission.CustomValidator) *WebhookBuilder { + blder.customValidator = validator + return blder +} + +// WithLogConstructor overrides the webhook's LogConstructor. +func (blder *WebhookBuilder) WithLogConstructor(logConstructor func(base logr.Logger, req *admission.Request) logr.Logger) *WebhookBuilder { + blder.logConstructor = logConstructor + return blder +} + +// RecoverPanic indicates whether panics caused by the webhook should be recovered. +// Defaults to true. +func (blder *WebhookBuilder) RecoverPanic(recoverPanic bool) *WebhookBuilder { + blder.recoverPanic = &recoverPanic + return blder +} + +// Complete builds the webhook. +func (blder *WebhookBuilder) Complete() error { + // Set the Config + blder.loadRestConfig() + + // Configure the default LogConstructor + blder.setLogConstructor() + + // Set the Webhook if needed + return blder.registerWebhooks() +} + +func (blder *WebhookBuilder) loadRestConfig() { + if blder.config == nil { + blder.config = blder.mgr.GetConfig() + } +} + +func (blder *WebhookBuilder) setLogConstructor() { + if blder.logConstructor == nil { + blder.logConstructor = func(base logr.Logger, req *admission.Request) logr.Logger { + log := base.WithValues( + "webhookGroup", blder.gvk.Group, + "webhookKind", blder.gvk.Kind, + ) + if req != nil { + return log.WithValues( + blder.gvk.Kind, klog.KRef(req.Namespace, req.Name), + "namespace", req.Namespace, "name", req.Name, + "resource", req.Resource, "user", req.UserInfo.Username, + "requestID", req.UID, + ) + } + return log + } + } +} + +func (blder *WebhookBuilder) registerWebhooks() error { + typ, err := blder.getType() + if err != nil { + return err + } + + blder.gvk, err = apiutil.GVKForObject(typ, blder.mgr.GetScheme()) + if err != nil { + return err + } + + // Register webhook(s) for type + blder.registerDefaultingWebhook() + blder.registerValidatingWebhook() + + err = blder.registerConversionWebhook() + if err != nil { + return err + } + return blder.err +} + +// registerDefaultingWebhook registers a defaulting webhook if necessary. +func (blder *WebhookBuilder) registerDefaultingWebhook() { + mwh := blder.getDefaultingWebhook() + if mwh != nil { + mwh.LogConstructor = blder.logConstructor + path := generateMutatePath(blder.gvk) + + // Checking if the path is already registered. + // If so, just skip it. + if !blder.isAlreadyHandled(path) { + log := blder.mgr.GetLogger() + log.Info("Registering a mutating webhook", + "GVK", blder.gvk, + "path", path) + blder.mgr.GetWebhookServer().Register(path, mwh) + } + } +} + +func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook { + if handler := blder.mutationHandler; handler != nil { + w := &admission.Webhook{ + Handler: handler, + } + if blder.recoverPanic != nil { + w = w.WithRecoverPanic(*blder.recoverPanic) + } + return w + } + return nil +} + +// registerValidatingWebhook registers a validating webhook if necessary. +func (blder *WebhookBuilder) registerValidatingWebhook() { + vwh := blder.getValidatingWebhook() + if vwh != nil { + vwh.LogConstructor = blder.logConstructor + path := generateValidatePath(blder.gvk) + + // Checking if the path is already registered. + // If so, just skip it. + if !blder.isAlreadyHandled(path) { + log := blder.mgr.GetLogger() + log.Info("Registering a validating webhook", + "GVK", blder.gvk, + "path", path) + blder.mgr.GetWebhookServer().Register(path, vwh) + } + } +} + +func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook { + if validator := blder.customValidator; validator != nil { + w := admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, validator) + if blder.recoverPanic != nil { + w = w.WithRecoverPanic(*blder.recoverPanic) + } + return w + } + return nil +} + +func (blder *WebhookBuilder) registerConversionWebhook() error { + log := blder.mgr.GetLogger() + ok, err := conversion.IsConvertible(blder.mgr.GetScheme(), blder.apiType) + if err != nil { + log.Error(err, "conversion check failed", "GVK", blder.gvk) + return err + } + if ok { + if !blder.isAlreadyHandled("/convert") { + blder.mgr.GetWebhookServer().Register("/convert", conversion.NewWebhookHandler(blder.mgr.GetScheme())) + } + log.Info("Conversion webhook enabled", "GVK", blder.gvk) + } + + return nil +} + +func (blder *WebhookBuilder) getType() (runtime.Object, error) { + if blder.apiType != nil { + return blder.apiType, nil + } + return nil, errors.New("For() must be called with a valid object") +} + +func (blder *WebhookBuilder) isAlreadyHandled(path string) bool { + if blder.mgr.GetWebhookServer().WebhookMux() == nil { + return false + } + h, p := blder.mgr.GetWebhookServer().WebhookMux().Handler(&http.Request{URL: &url.URL{Path: path}}) + if p == path && h != nil { + return true + } + return false +} + +func generateMutatePath(gvk schema.GroupVersionKind) string { + return "/mutate-" + strings.ReplaceAll(gvk.Group, ".", "-") + "-" + + gvk.Version + "-" + strings.ToLower(gvk.Kind) +} + +func generateValidatePath(gvk schema.GroupVersionKind) string { + return "/validate-" + strings.ReplaceAll(gvk.Group, ".", "-") + "-" + + gvk.Version + "-" + strings.ToLower(gvk.Kind) +} diff --git a/pkg/controller/jobframework/webhook/defaulter.go b/pkg/controller/jobframework/webhook/defaulter.go new file mode 100644 index 0000000000..ee94e730ee --- /dev/null +++ b/pkg/controller/jobframework/webhook/defaulter.go @@ -0,0 +1,61 @@ +/* +Copyright 2024 The Kubernetes 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 webhook + +import ( + "context" + + jsonpatch "gomodules.xyz/jsonpatch/v2" + "k8s.io/apimachinery/pkg/runtime" + + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// WithLosslessDefaulter creates a new Handler for a CustomDefaulter interface that **drops** remove operations, +// which are typically the result of new API fields not present in Kueue libraries. +func WithLosslessDefaulter(scheme *runtime.Scheme, obj runtime.Object, defaulter admission.CustomDefaulter) admission.Handler { + return &losslessDefaulter{ + Handler: admission.WithCustomDefaulter(scheme, obj, defaulter).Handler, + } +} + +type losslessDefaulter struct { + admission.Handler +} + +// Handle handles admission requests, **dropping** remove operations from patches produced by controller-runtime. +// The controller-runtime handler works by creating a jsondiff from the raw object and the marshalled +// version of the object modified by the defaulter. This generates "remove" operations for fields +// that are not present in the go types, which can occur when Kueue libraries are behind the latest +// released CRDs. +// Dropping the "remove" operations is safe because Kueue's job mutators never remove fields. +func (h *losslessDefaulter) Handle(ctx context.Context, req admission.Request) admission.Response { + response := h.Handler.Handle(ctx, req) + if response.Allowed { + var patches []jsonpatch.Operation + for _, p := range response.Patches { + if p.Operation != "remove" { + patches = append(patches, p) + } + } + if len(patches) == 0 { + response.PatchType = nil + } + response.Patches = patches + } + return response +} diff --git a/pkg/controller/jobframework/webhook/defaulter_test.go b/pkg/controller/jobframework/webhook/defaulter_test.go new file mode 100644 index 0000000000..d7e8b1fc53 --- /dev/null +++ b/pkg/controller/jobframework/webhook/defaulter_test.go @@ -0,0 +1,99 @@ +/* +Copyright 2024 The Kubernetes 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 webhook + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + jsonpatch "gomodules.xyz/jsonpatch/v2" + admissionv1 "k8s.io/api/admission/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/scheme" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +var ( + testResourceKind = "TestResource" + testResourceGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: testResourceKind} +) + +type TestResource struct { + Foo string `json:"foo,omitempty"` +} + +func (d *TestResource) GetObjectKind() schema.ObjectKind { return d } +func (d *TestResource) DeepCopyObject() runtime.Object { + return &TestResource{ + Foo: d.Foo, + } +} + +func (d *TestResource) GroupVersionKind() schema.GroupVersionKind { + return testResourceGVK +} + +func (d *TestResource) SetGroupVersionKind(gvk schema.GroupVersionKind) {} + +type TestCustomDefaulter struct{} + +func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error { + d := obj.(*TestResource) + if d.Foo == "" { + d.Foo = "bar" + } + return nil +} + +func TestLossLessDefaulter(t *testing.T) { + sch := runtime.NewScheme() + builder := scheme.Builder{GroupVersion: testResourceGVK.GroupVersion()} + builder.Register(&TestResource{}) + if err := builder.AddToScheme(sch); err != nil { + t.Fatalf("Couldn't add types to scheme: %v", err) + } + + handler := WithLosslessDefaulter(sch, &TestResource{}, &TestCustomDefaulter{}) + + req := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Kind: metav1.GroupVersionKind(testResourceGVK), + Object: runtime.RawExtension{ + // This raw object has a field not defined in the go type. + // controller-runtime CustomDefaulter would have added a remove operation for it. + Raw: []byte(`{"baz": "qux"}`), + }, + }, + } + resp := handler.Handle(context.Background(), req) + if !resp.Allowed { + t.Errorf("Response not allowed") + } + wantPatches := []jsonpatch.Operation{ + { + Operation: "add", + Path: "/foo", + Value: "bar", + }, + } + if diff := cmp.Diff(wantPatches, resp.Patches); diff != "" { + t.Errorf("Unexpected patches (-want, +got): %s", diff) + } +} diff --git a/pkg/controller/jobs/deployment/deployment_webhook.go b/pkg/controller/jobs/deployment/deployment_webhook.go index ab4fef58fa..a39c637765 100644 --- a/pkg/controller/jobs/deployment/deployment_webhook.go +++ b/pkg/controller/jobs/deployment/deployment_webhook.go @@ -26,11 +26,11 @@ import ( "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "sigs.k8s.io/kueue/pkg/controller/constants" "sigs.k8s.io/kueue/pkg/controller/jobframework" + "sigs.k8s.io/kueue/pkg/controller/jobframework/webhook" ) type Webhook struct { @@ -44,16 +44,17 @@ func SetupWebhook(mgr ctrl.Manager, opts ...jobframework.Option) error { client: mgr.GetClient(), manageJobsWithoutQueueName: options.ManageJobsWithoutQueueName, } - return ctrl.NewWebhookManagedBy(mgr). - For(&appsv1.Deployment{}). - WithDefaulter(wh). + obj := &appsv1.Deployment{} + return webhook.WebhookManagedBy(mgr). + For(obj). + WithMutationHandler(webhook.WithLosslessDefaulter(mgr.GetScheme(), obj, wh)). WithValidator(wh). Complete() } // +kubebuilder:webhook:path=/mutate-apps-v1-deployment,mutating=true,failurePolicy=fail,sideEffects=None,groups="apps",resources=deployments,verbs=create,versions=v1,name=mdeployment.kb.io,admissionReviewVersions=v1 -var _ webhook.CustomDefaulter = &Webhook{} +var _ admission.CustomDefaulter = &Webhook{} func (wh *Webhook) Default(ctx context.Context, obj runtime.Object) error { d := fromObject(obj) @@ -73,7 +74,7 @@ func (wh *Webhook) Default(ctx context.Context, obj runtime.Object) error { // +kubebuilder:webhook:path=/validate-apps-v1-deployment,mutating=false,failurePolicy=fail,sideEffects=None,groups="apps",resources=deployments,verbs=create;update,versions=v1,name=vdeployment.kb.io,admissionReviewVersions=v1 -var _ webhook.CustomValidator = &Webhook{} +var _ admission.CustomValidator = &Webhook{} func (wh *Webhook) ValidateCreate(context.Context, runtime.Object) (warnings admission.Warnings, err error) { return nil, nil diff --git a/pkg/controller/jobs/job/job_webhook.go b/pkg/controller/jobs/job/job_webhook.go index ec48fbdd5c..13e1ca4baa 100644 --- a/pkg/controller/jobs/job/job_webhook.go +++ b/pkg/controller/jobs/job/job_webhook.go @@ -28,13 +28,13 @@ import ( "k8s.io/klog/v2" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "sigs.k8s.io/kueue/apis/kueue/v1alpha1" "sigs.k8s.io/kueue/pkg/cache" "sigs.k8s.io/kueue/pkg/controller/constants" "sigs.k8s.io/kueue/pkg/controller/jobframework" + "sigs.k8s.io/kueue/pkg/controller/jobframework/webhook" "sigs.k8s.io/kueue/pkg/features" "sigs.k8s.io/kueue/pkg/queue" "sigs.k8s.io/kueue/pkg/util/kubeversion" @@ -61,16 +61,17 @@ func SetupWebhook(mgr ctrl.Manager, opts ...jobframework.Option) error { queues: options.Queues, cache: options.Cache, } - return ctrl.NewWebhookManagedBy(mgr). - For(&batchv1.Job{}). - WithDefaulter(wh). + obj := &batchv1.Job{} + return webhook.WebhookManagedBy(mgr). + For(obj). + WithMutationHandler(webhook.WithLosslessDefaulter(mgr.GetScheme(), obj, wh)). WithValidator(wh). Complete() } // +kubebuilder:webhook:path=/mutate-batch-v1-job,mutating=true,failurePolicy=fail,sideEffects=None,groups=batch,resources=jobs,verbs=create,versions=v1,name=mjob.kb.io,admissionReviewVersions=v1 -var _ webhook.CustomDefaulter = &JobWebhook{} +var _ admission.CustomDefaulter = &JobWebhook{} // Default implements webhook.CustomDefaulter so a webhook will be registered for the type func (w *JobWebhook) Default(ctx context.Context, obj runtime.Object) error { @@ -109,7 +110,7 @@ func canDefaultManagedBy(jobSpecManagedBy *string) bool { // +kubebuilder:webhook:path=/validate-batch-v1-job,mutating=false,failurePolicy=fail,sideEffects=None,groups=batch,resources=jobs,verbs=create;update,versions=v1,name=vjob.kb.io,admissionReviewVersions=v1 -var _ webhook.CustomValidator = &JobWebhook{} +var _ admission.CustomValidator = &JobWebhook{} // ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type func (w *JobWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { diff --git a/pkg/controller/jobs/jobset/jobset_webhook.go b/pkg/controller/jobs/jobset/jobset_webhook.go index b51627a142..d2d3b6cb4d 100644 --- a/pkg/controller/jobs/jobset/jobset_webhook.go +++ b/pkg/controller/jobs/jobset/jobset_webhook.go @@ -23,7 +23,6 @@ import ( "k8s.io/klog/v2" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" jobsetapi "sigs.k8s.io/jobset/api/jobset/v1alpha2" @@ -31,6 +30,7 @@ import ( "sigs.k8s.io/kueue/pkg/cache" "sigs.k8s.io/kueue/pkg/controller/constants" "sigs.k8s.io/kueue/pkg/controller/jobframework" + "sigs.k8s.io/kueue/pkg/controller/jobframework/webhook" "sigs.k8s.io/kueue/pkg/features" "sigs.k8s.io/kueue/pkg/queue" ) @@ -49,16 +49,17 @@ func SetupJobSetWebhook(mgr ctrl.Manager, opts ...jobframework.Option) error { queues: options.Queues, cache: options.Cache, } - return ctrl.NewWebhookManagedBy(mgr). - For(&jobsetapi.JobSet{}). - WithDefaulter(wh). + obj := &jobsetapi.JobSet{} + return webhook.WebhookManagedBy(mgr). + For(obj). + WithMutationHandler(webhook.WithLosslessDefaulter(mgr.GetScheme(), obj, wh)). WithValidator(wh). Complete() } // +kubebuilder:webhook:path=/mutate-jobset-x-k8s-io-v1alpha2-jobset,mutating=true,failurePolicy=fail,sideEffects=None,groups=jobset.x-k8s.io,resources=jobsets,verbs=create,versions=v1alpha2,name=mjobset.kb.io,admissionReviewVersions=v1 -var _ webhook.CustomDefaulter = &JobSetWebhook{} +var _ admission.CustomDefaulter = &JobSetWebhook{} // Default implements webhook.CustomDefaulter so a webhook will be registered for the type func (w *JobSetWebhook) Default(ctx context.Context, obj runtime.Object) error { @@ -97,7 +98,7 @@ func canDefaultManagedBy(jobSetSpecManagedBy *string) bool { // +kubebuilder:webhook:path=/validate-jobset-x-k8s-io-v1alpha2-jobset,mutating=false,failurePolicy=fail,sideEffects=None,groups=jobset.x-k8s.io,resources=jobsets,verbs=create;update,versions=v1alpha2,name=vjobset.kb.io,admissionReviewVersions=v1 -var _ webhook.CustomValidator = &JobSetWebhook{} +var _ admission.CustomValidator = &JobSetWebhook{} // ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type func (w *JobSetWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { diff --git a/pkg/controller/jobs/pod/pod_webhook.go b/pkg/controller/jobs/pod/pod_webhook.go index f805169fd8..9d9aa41c85 100644 --- a/pkg/controller/jobs/pod/pod_webhook.go +++ b/pkg/controller/jobs/pod/pod_webhook.go @@ -31,12 +31,12 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" configapi "sigs.k8s.io/kueue/apis/config/v1beta1" "sigs.k8s.io/kueue/pkg/constants" "sigs.k8s.io/kueue/pkg/controller/jobframework" + "sigs.k8s.io/kueue/pkg/controller/jobframework/webhook" ) const ( @@ -80,9 +80,10 @@ func SetupWebhook(mgr ctrl.Manager, opts ...jobframework.Option) error { namespaceSelector: podOpts.NamespaceSelector, podSelector: podOpts.PodSelector, } - return ctrl.NewWebhookManagedBy(mgr). - For(&corev1.Pod{}). - WithDefaulter(wh). + obj := &corev1.Pod{} + return webhook.WebhookManagedBy(mgr). + For(obj). + WithMutationHandler(webhook.WithLosslessDefaulter(mgr.GetScheme(), obj, wh)). WithValidator(wh). Complete() } @@ -102,7 +103,7 @@ func getPodOptions(integrationOpts map[string]any) (*configapi.PodIntegrationOpt // +kubebuilder:webhook:path=/mutate--v1-pod,mutating=true,failurePolicy=fail,sideEffects=None,groups="",resources=pods,verbs=create,versions=v1,name=mpod.kb.io,admissionReviewVersions=v1 // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch -var _ webhook.CustomDefaulter = &PodWebhook{} +var _ admission.CustomDefaulter = &PodWebhook{} func containersShape(containers []corev1.Container) (result []map[string]interface{}) { for _, c := range containers { @@ -196,7 +197,7 @@ func (w *PodWebhook) Default(ctx context.Context, obj runtime.Object) error { // +kubebuilder:webhook:path=/validate--v1-pod,mutating=false,failurePolicy=fail,sideEffects=None,groups="",resources=pods,verbs=create;update,versions=v1,name=vpod.kb.io,admissionReviewVersions=v1 -var _ webhook.CustomValidator = &PodWebhook{} +var _ admission.CustomValidator = &PodWebhook{} func (w *PodWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { var warnings admission.Warnings diff --git a/pkg/controller/jobs/raycluster/raycluster_webhook.go b/pkg/controller/jobs/raycluster/raycluster_webhook.go index 2976ca57dc..5b9f93edd1 100644 --- a/pkg/controller/jobs/raycluster/raycluster_webhook.go +++ b/pkg/controller/jobs/raycluster/raycluster_webhook.go @@ -24,10 +24,10 @@ import ( "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "sigs.k8s.io/kueue/pkg/controller/jobframework" + "sigs.k8s.io/kueue/pkg/controller/jobframework/webhook" ) type RayClusterWebhook struct { @@ -43,16 +43,17 @@ func SetupRayClusterWebhook(mgr ctrl.Manager, opts ...jobframework.Option) error wh := &RayClusterWebhook{ manageJobsWithoutQueueName: options.ManageJobsWithoutQueueName, } - return ctrl.NewWebhookManagedBy(mgr). - For(&rayv1.RayCluster{}). - WithDefaulter(wh). + obj := &rayv1.RayCluster{} + return webhook.WebhookManagedBy(mgr). + For(obj). + WithMutationHandler(webhook.WithLosslessDefaulter(mgr.GetScheme(), obj, wh)). WithValidator(wh). Complete() } // +kubebuilder:webhook:path=/mutate-ray-io-v1-raycluster,mutating=true,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayclusters,verbs=create,versions=v1,name=mraycluster.kb.io,admissionReviewVersions=v1 -var _ webhook.CustomDefaulter = &RayClusterWebhook{} +var _ admission.CustomDefaulter = &RayClusterWebhook{} // Default implements webhook.CustomDefaulter so a webhook will be registered for the type func (w *RayClusterWebhook) Default(ctx context.Context, obj runtime.Object) error { @@ -65,7 +66,7 @@ func (w *RayClusterWebhook) Default(ctx context.Context, obj runtime.Object) err // +kubebuilder:webhook:path=/validate-ray-io-v1-raycluster,mutating=false,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayclusters,verbs=create;update,versions=v1,name=vraycluster.kb.io,admissionReviewVersions=v1 -var _ webhook.CustomValidator = &RayClusterWebhook{} +var _ admission.CustomValidator = &RayClusterWebhook{} // ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type func (w *RayClusterWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { diff --git a/pkg/controller/jobs/rayjob/rayjob_webhook.go b/pkg/controller/jobs/rayjob/rayjob_webhook.go index 2c53cdc7d4..fa216b6749 100644 --- a/pkg/controller/jobs/rayjob/rayjob_webhook.go +++ b/pkg/controller/jobs/rayjob/rayjob_webhook.go @@ -27,10 +27,10 @@ import ( "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "sigs.k8s.io/kueue/pkg/controller/jobframework" + "sigs.k8s.io/kueue/pkg/controller/jobframework/webhook" ) type RayJobWebhook struct { @@ -43,16 +43,17 @@ func SetupRayJobWebhook(mgr ctrl.Manager, opts ...jobframework.Option) error { wh := &RayJobWebhook{ manageJobsWithoutQueueName: options.ManageJobsWithoutQueueName, } - return ctrl.NewWebhookManagedBy(mgr). - For(&rayv1.RayJob{}). - WithDefaulter(wh). + obj := &rayv1.RayJob{} + return webhook.WebhookManagedBy(mgr). + For(obj). + WithMutationHandler(webhook.WithLosslessDefaulter(mgr.GetScheme(), obj, wh)). WithValidator(wh). Complete() } // +kubebuilder:webhook:path=/mutate-ray-io-v1-rayjob,mutating=true,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayjobs,verbs=create,versions=v1,name=mrayjob.kb.io,admissionReviewVersions=v1 -var _ webhook.CustomDefaulter = &RayJobWebhook{} +var _ admission.CustomDefaulter = &RayJobWebhook{} // Default implements webhook.CustomDefaulter so a webhook will be registered for the type func (w *RayJobWebhook) Default(ctx context.Context, obj runtime.Object) error { @@ -65,7 +66,7 @@ func (w *RayJobWebhook) Default(ctx context.Context, obj runtime.Object) error { // +kubebuilder:webhook:path=/validate-ray-io-v1-rayjob,mutating=false,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayjobs,verbs=create;update,versions=v1,name=vrayjob.kb.io,admissionReviewVersions=v1 -var _ webhook.CustomValidator = &RayJobWebhook{} +var _ admission.CustomValidator = &RayJobWebhook{} // ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type func (w *RayJobWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {