Hi @jaronoff97 I am an Outreachy applicant, and I would like to work on this issue. I'll review the project and keep you updated on my progress. In the meantime, I would appreciate any insights or guidance you could provide to help me address this issue.

@mercybassey sounds good! one issue with this is that feature went beta in Kubernetes 1.25 and stable in kubernetes 1.29 (link) and right now our minimum supported version is 1.23. We will probably need to bump that minimum version as part of this. We have an open issue here for that #2798

@jaronoff97 Thank you for the update! I’d like to clarify if I can go ahead with migrating the validation rules to the CRDs while keeping an eye on the progress of issue #2798 regarding the version bump.

Assuming the x-kubernetes-validations field is ignored on K8s <1.25 (it should be, but we need to verify), then I think we can add these validations, without removing existing ones from the webhook. There isn't really any downside to having both for users. It does represent additional maintenance burden for us, but I wouldn't expect it to be very noticable.

@mercybassey if you want to work on this, I'd limit the scope to just adding the existing webhook validations as validation rules, without removing the former. You can start by familiarizing yourself with the webhook logic, how we define and annotate our CRDs using kubebuilder, and kubebuilder's own documentation. What you want for CRD validation rules is the +kubebuilder:validation:XValidation marker.

Here's also a tutorial showing how to use CEL with kubebuilder in practice.

Assuming the x-kubernetes-validations field is ignored on K8s <1.25 (it should be, but we need to verify), then I think we can add these validations, without removing existing ones from the webhook. There isn't really any downside to having both for users. It does represent additional maintenance burden for us, but I wouldn't expect it to be very noticable.

@mercybassey if you want to work on this, I'd limit the scope to just adding the existing webhook validations as validation rules, without removing the former. You can start by familiarizing yourself with the webhook logic, how we define and annotate our CRDs using kubebuilder, and kubebuilder's own documentation. What you want for CRD validation rules is the +kubebuilder:validation:XValidation marker.

Here's also a tutorial showing how to use CEL with kubebuilder in practice.

Thank you for your guidance. I'll start working on it right away.

@mercybassey Are you still planning on contributing to this issue? If not, let me know as I'd like to take a shot at this ;)

@mercybassey Are you still planning on contributing to this issue? If not, let me know as I'd like to take a shot at this ;)

You can go ahead.

I've created a first draft PR to illustrate my solution. I'd like some feedback on the work I've done so far before continuing, just to know if I'm on the right track: #3580. The changes I've implemented so far are equivalent to logic found in the collector_webhook.go file, specifically in these lines: L62, L65, L187 and L192.

If I understood correctly, given that #2798 has been closed, we should also remove the equivalent logic from the webhooks, is that correct?

If someone can confirm I'm on the right path, next steps should be to move as much logic as possible from these files into their respective CRD definitions. I'd also like to have some input on whether it would be better to have one PR for each file or do everything at once in a single PR.

• apis/v1alpha1/instrumentation_webhook.go
• apis/v1alpha1/opampbridge_webhook.go
• apis/v1alpha1/targetallocator_webhook.go
• apis/v1beta1/collector_webhook.go

cc @jaronoff97 @swiatekm

@jbiers #3580 looks good to me, that's exactly what we want. If you want to make similar changes for other CRDs, I'd prefer to review a separate PR for each CRD, and possibly make the changes even more granular depending on how complex they are. Keep in mind that we'll also need to test these rules, which will require a real API Server, and therefore also envtest.

If I understood correctly, given that #2798 has been closed, we should also remove the equivalent logic from the webhooks, is that correct?

That is not the case. We currently support K8s 1.23, which doesn't have CEL at all. For the time being, we'll need to maintain both approaches.