From 24850964f31ca1a8e222ddc9d2751e3472b4375a Mon Sep 17 00:00:00 2001 From: ChristianZaccaria Date: Wed, 17 Apr 2024 18:01:35 +0100 Subject: [PATCH 1/3] Create RayCluster ValidatingWebhook --- PROJECT | 1 + config/openshift/kustomization.yaml | 4 +- ...ch.yaml => webhookcainjection_mpatch.yaml} | 0 .../openshift/webhookcainjection_vpatch.yaml | 7 ++++ config/webhook/kustomization.yaml | 2 + config/webhook/manifests.yaml | 27 +++++++++++++ pkg/controllers/raycluster_webhook.go | 39 +++++++++++++++++++ 7 files changed, 78 insertions(+), 2 deletions(-) rename config/openshift/{webhookcainjection_patch.yaml => webhookcainjection_mpatch.yaml} (100%) create mode 100644 config/openshift/webhookcainjection_vpatch.yaml diff --git a/PROJECT b/PROJECT index 0fc5562e2..965a62fd1 100644 --- a/PROJECT +++ b/PROJECT @@ -19,5 +19,6 @@ resources: version: v1 webhooks: defaulting: true + validation: true webhookVersion: v1 version: "3" diff --git a/config/openshift/kustomization.yaml b/config/openshift/kustomization.yaml index 22e168162..b39da0736 100644 --- a/config/openshift/kustomization.yaml +++ b/config/openshift/kustomization.yaml @@ -6,7 +6,6 @@ namespace: openshift-operators # "wordpress" becomes "alices-wordpress". # Note that it should also match with the prefix (text before '-') of the namespace # field above. -namePrefix: codeflare-operator- # Labels to add to all resources and selectors. commonLabels: @@ -19,4 +18,5 @@ bases: patches: - path: manager_webhook_patch.yaml -- path: webhookcainjection_patch.yaml +- path: webhookcainjection_mpatch.yaml +- path: webhookcainjection_vpatch.yaml diff --git a/config/openshift/webhookcainjection_patch.yaml b/config/openshift/webhookcainjection_mpatch.yaml similarity index 100% rename from config/openshift/webhookcainjection_patch.yaml rename to config/openshift/webhookcainjection_mpatch.yaml diff --git a/config/openshift/webhookcainjection_vpatch.yaml b/config/openshift/webhookcainjection_vpatch.yaml new file mode 100644 index 000000000..91f741a05 --- /dev/null +++ b/config/openshift/webhookcainjection_vpatch.yaml @@ -0,0 +1,7 @@ +# This patch add annotation to admission webhook config +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: validating-webhook-configuration + annotations: + service.beta.openshift.io/inject-cabundle: "true" diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml index 9cf26134e..412cee110 100644 --- a/config/webhook/kustomization.yaml +++ b/config/webhook/kustomization.yaml @@ -1,3 +1,5 @@ +namePrefix: codeflare-operator- + resources: - manifests.yaml - service.yaml diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index faf91696b..c14976cfe 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -25,3 +25,30 @@ webhooks: resources: - rayclusters sideEffects: None +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + creationTimestamp: null + name: validating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-ray-io-v1-raycluster + failurePolicy: Fail + name: vraycluster.kb.io + rules: + - apiGroups: + - ray.io + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - rayclusters + sideEffects: None diff --git a/pkg/controllers/raycluster_webhook.go b/pkg/controllers/raycluster_webhook.go index 0cd635040..d0c046941 100644 --- a/pkg/controllers/raycluster_webhook.go +++ b/pkg/controllers/raycluster_webhook.go @@ -23,10 +23,12 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/project-codeflare/codeflare-operator/pkg/config" ) @@ -40,16 +42,24 @@ func SetupRayClusterWebhookWithManager(mgr ctrl.Manager, cfg *config.KubeRayConf WithDefaulter(&rayClusterDefaulter{ Config: cfg, }). + WithValidator(&rayClusterWebhook{ + Config: cfg, + }). Complete() } // +kubebuilder:webhook:path=/mutate-ray-io-v1-raycluster,mutating=true,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayclusters,verbs=create;update,versions=v1,name=mraycluster.kb.io,admissionReviewVersions=v1 +// +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 type rayClusterDefaulter struct { Config *config.KubeRayConfiguration } +type rayClusterWebhook struct { + Config *config.KubeRayConfiguration +} var _ webhook.CustomDefaulter = &rayClusterDefaulter{} +var _ webhook.CustomValidator = &rayClusterWebhook{} // Default implements webhook.Defaulter so a webhook will be registered for the type func (r *rayClusterDefaulter) Default(ctx context.Context, obj runtime.Object) error { @@ -132,3 +142,32 @@ func (r *rayClusterDefaulter) Default(ctx context.Context, obj runtime.Object) e return nil } + +func (v *rayClusterWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + raycluster := obj.(*rayv1.RayCluster) + var warnings admission.Warnings + var allErrors field.ErrorList + specPath := field.NewPath("spec") + + if pointer.BoolDeref(raycluster.Spec.HeadGroupSpec.EnableIngress, false) { + rayclusterlog.Info("Creating RayCluster resources with EnableIngress set to true or unspecified is not allowed") + allErrors = append(allErrors, field.Invalid(specPath.Child("headGroupSpec").Child("enableIngress"), raycluster.Spec.HeadGroupSpec.EnableIngress, "creating RayCluster resources with EnableIngress set to true or unspecified is not allowed")) + } + + return warnings, allErrors.ToAggregate() +} + +func (v *rayClusterWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { + newRayCluster := newObj.(*rayv1.RayCluster) + if !newRayCluster.DeletionTimestamp.IsZero() { + // Object is being deleted, skip validations + return nil, nil + } + warnings, err := v.ValidateCreate(ctx, newRayCluster) + return warnings, err +} + +func (v *rayClusterWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + // Optional: Add delete validation logic here + return nil, nil +} From a05d98d40648f399268ca2cf63bcc5c9a02b9db1 Mon Sep 17 00:00:00 2001 From: ChristianZaccaria Date: Thu, 18 Apr 2024 10:16:15 +0100 Subject: [PATCH 2/3] Remove Defaulter type in webhook --- pkg/controllers/raycluster_webhook.go | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/pkg/controllers/raycluster_webhook.go b/pkg/controllers/raycluster_webhook.go index d0c046941..bc003d4d2 100644 --- a/pkg/controllers/raycluster_webhook.go +++ b/pkg/controllers/raycluster_webhook.go @@ -27,7 +27,6 @@ import ( "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/project-codeflare/codeflare-operator/pkg/config" @@ -39,7 +38,7 @@ var rayclusterlog = logf.Log.WithName("raycluster-resource") func SetupRayClusterWebhookWithManager(mgr ctrl.Manager, cfg *config.KubeRayConfiguration) error { return ctrl.NewWebhookManagedBy(mgr). For(&rayv1.RayCluster{}). - WithDefaulter(&rayClusterDefaulter{ + WithDefaulter(&rayClusterWebhook{ Config: cfg, }). WithValidator(&rayClusterWebhook{ @@ -51,21 +50,15 @@ func SetupRayClusterWebhookWithManager(mgr ctrl.Manager, cfg *config.KubeRayConf // +kubebuilder:webhook:path=/mutate-ray-io-v1-raycluster,mutating=true,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayclusters,verbs=create;update,versions=v1,name=mraycluster.kb.io,admissionReviewVersions=v1 // +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 -type rayClusterDefaulter struct { - Config *config.KubeRayConfiguration -} type rayClusterWebhook struct { Config *config.KubeRayConfiguration } -var _ webhook.CustomDefaulter = &rayClusterDefaulter{} -var _ webhook.CustomValidator = &rayClusterWebhook{} - // Default implements webhook.Defaulter so a webhook will be registered for the type -func (r *rayClusterDefaulter) Default(ctx context.Context, obj runtime.Object) error { +func (w *rayClusterWebhook) Default(ctx context.Context, obj runtime.Object) error { raycluster := obj.(*rayv1.RayCluster) - if !pointer.BoolDeref(r.Config.RayDashboardOAuthEnabled, true) { + if !pointer.BoolDeref(w.Config.RayDashboardOAuthEnabled, true) { return nil } @@ -143,7 +136,7 @@ func (r *rayClusterDefaulter) Default(ctx context.Context, obj runtime.Object) e return nil } -func (v *rayClusterWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { +func (w *rayClusterWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { raycluster := obj.(*rayv1.RayCluster) var warnings admission.Warnings var allErrors field.ErrorList @@ -157,17 +150,17 @@ func (v *rayClusterWebhook) ValidateCreate(ctx context.Context, obj runtime.Obje return warnings, allErrors.ToAggregate() } -func (v *rayClusterWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { +func (w *rayClusterWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { newRayCluster := newObj.(*rayv1.RayCluster) if !newRayCluster.DeletionTimestamp.IsZero() { // Object is being deleted, skip validations return nil, nil } - warnings, err := v.ValidateCreate(ctx, newRayCluster) + warnings, err := w.ValidateCreate(ctx, newRayCluster) return warnings, err } -func (v *rayClusterWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { +func (w *rayClusterWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { // Optional: Add delete validation logic here return nil, nil } From a849c9b4e33122e7bf871b142a074af6265200d9 Mon Sep 17 00:00:00 2001 From: ChristianZaccaria Date: Thu, 18 Apr 2024 10:32:44 +0100 Subject: [PATCH 3/3] Use single rayClusterWebhook instance --- pkg/controllers/raycluster_webhook.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/controllers/raycluster_webhook.go b/pkg/controllers/raycluster_webhook.go index bc003d4d2..8d06eebec 100644 --- a/pkg/controllers/raycluster_webhook.go +++ b/pkg/controllers/raycluster_webhook.go @@ -27,6 +27,7 @@ import ( "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/project-codeflare/codeflare-operator/pkg/config" @@ -36,14 +37,13 @@ import ( var rayclusterlog = logf.Log.WithName("raycluster-resource") func SetupRayClusterWebhookWithManager(mgr ctrl.Manager, cfg *config.KubeRayConfiguration) error { + rayClusterWebhookInstance := &rayClusterWebhook{ + Config: cfg, + } return ctrl.NewWebhookManagedBy(mgr). For(&rayv1.RayCluster{}). - WithDefaulter(&rayClusterWebhook{ - Config: cfg, - }). - WithValidator(&rayClusterWebhook{ - Config: cfg, - }). + WithDefaulter(rayClusterWebhookInstance). + WithValidator(rayClusterWebhookInstance). Complete() } @@ -54,6 +54,9 @@ type rayClusterWebhook struct { Config *config.KubeRayConfiguration } +var _ webhook.CustomDefaulter = &rayClusterWebhook{} +var _ webhook.CustomValidator = &rayClusterWebhook{} + // Default implements webhook.Defaulter so a webhook will be registered for the type func (w *rayClusterWebhook) Default(ctx context.Context, obj runtime.Object) error { raycluster := obj.(*rayv1.RayCluster)