use CEL for CR validation by Madhu-1 · Pull Request #459 · csi-addons/kubernetes-csi-addons

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

 	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

 		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 <madhupr007@gmail.com>

added csi-addons.clusterserviceversion to gitignore as its not pushed to the repo. Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

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 <madhupr007@gmail.com>

mergify Bot requested review from Rakshith-R, nixpanic, 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 <madhupr007@gmail.com>

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

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
mergify[bot] merged 3 commits into
csi-addons:mainfrom
Madhu-1:remove-webhook-use-cel

Madhu-1 commented Sep 21, 2023

Uh oh!

Rakshith-R left a comment

Uh oh!

nixpanic left a comment

Uh oh!

nixpanic Sep 26, 2023

Uh oh!

nixpanic Sep 26, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Madhu-1 commented Sep 21, 2023

Uh oh!

Rakshith-R left a comment

Choose a reason for hiding this comment

Uh oh!

nixpanic left a comment

Choose a reason for hiding this comment

Uh oh!

nixpanic Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

nixpanic Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants