use CEL for CR validation #459

Madhu-1 · 2023-09-21T13:09:01Z

CEL is the preferred way due to its advantages over webhooks and CEL is beta in 1.25 and will be GA in 1.29 we can avoid the burden of maintenance and code complexity of webhook if we move from webhook to CEL.

This PR removed all the webhook-related stuff and added CEL for CR validation.

Mode details about CEL at https://kubernetes.io/blog/2022/09/29/enforce-immutability-using-cel/

Rakshith-R

LGTM,
thanks!

nixpanic

just a nit, looks good otherwise

nixpanic · 2023-09-26T13:12:47Z

cmd/manager/main.go

@@ -83,7 +83,7 @@ func main() {
 	flag.DurationVar(&cfg.ReclaimSpaceTimeout, "reclaim-space-timeout", cfg.ReclaimSpaceTimeout, "Timeout for reclaimspace operation")
 	flag.IntVar(&cfg.MaxConcurrentReconciles, "max-concurrent-reconciles", cfg.MaxConcurrentReconciles, "Maximum number of concurrent reconciles")
 	flag.StringVar(&cfg.Namespace, "namespace", cfg.Namespace, "Namespace where the CSIAddons pod is deployed")
-	flag.BoolVar(&enableAdmissionWebhooks, "enable-admission-webhooks", true, "Enable the admission webhooks")
+	flag.BoolVar(&enableAdmissionWebhooks, "enable-admission-webhooks", false, "[DEPRECATED] Enable the admission webhooks")


Please report a warning when the option is passed. Note the name of the option, and that the functionality has been removed.

nixpanic · 2023-09-26T13:13:45Z

cmd/manager/main.go

@@ -97,6 +97,10 @@ func main() {
 		return
 	}

+	if enableAdmissionWebhooks {


please squash this into the 1st commit

Pull request has been modified.

As webhooks are complicated and hard to maintain removing it from the repo and will use CEL for all the required validations. print deprecated log for webhook as webhook is disabled and print a log if its enabled. Signed-off-by: Madhu Rajanna <[email protected]>

added csi-addons.clusterserviceversion to gitignore as its not pushed to the repo. Signed-off-by: Madhu Rajanna <[email protected]>

use CEL to validate the CR fields as it is having multiple advantages over webhooks and also it already beta in 1.25 onwards and going to be GA in 1.29 More details about it at https://kubernetes.io/blog/2022/09/29/\ enforce-immutability-using-cel/ Signed-off-by: Madhu Rajanna <[email protected]>

mergify bot requested review from nixpanic, Rakshith-R, riya-singhal31 and yati1998 September 21, 2023 13:09

Rakshith-R previously approved these changes Sep 25, 2023

View reviewed changes

nixpanic reviewed Sep 26, 2023

View reviewed changes

Madhu-1 force-pushed the remove-webhook-use-cel branch from f63ea38 to 5a894b2 Compare September 26, 2023 13:21

Madhu-1 requested a review from nixpanic September 26, 2023 13:22

nixpanic requested a review from Rakshith-R September 28, 2023 10:58

nixpanic approved these changes Sep 28, 2023

View reviewed changes

Rakshith-R approved these changes Sep 28, 2023

View reviewed changes

Madhu-1 added 3 commits September 28, 2023 11:02

add csi-addons.clusterserviceversion to gitignore

2bb55f7

added csi-addons.clusterserviceversion to gitignore as its not pushed to the repo. Signed-off-by: Madhu Rajanna <[email protected]>

Madhu-1 force-pushed the remove-webhook-use-cel branch from 5a894b2 to 7689242 Compare September 28, 2023 11:02

mergify bot merged commit 1c3e27c into csi-addons:main Sep 28, 2023
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use CEL for CR validation #459

use CEL for CR validation #459

Madhu-1 commented Sep 21, 2023

Rakshith-R left a comment

nixpanic left a comment

nixpanic Sep 26, 2023

nixpanic Sep 26, 2023

use CEL for CR validation #459

use CEL for CR validation #459

Conversation

Madhu-1 commented Sep 21, 2023

Rakshith-R left a comment

Choose a reason for hiding this comment

nixpanic left a comment

Choose a reason for hiding this comment

nixpanic Sep 26, 2023

Choose a reason for hiding this comment

nixpanic Sep 26, 2023

Choose a reason for hiding this comment