fix: removing side effects from the ScaledObject webhook when the req…

…uest is in dry-run mode (kedacore#5307)

Signed-off-by: Rodrigo Fior Kuntzer <[email protected]>
Signed-off-by: anton.lysina <[email protected]>

CHANGELOG.md

@@ -81,6 +81,7 @@ Here is an overview of all new **experimental** features:
 - **General**: Fix otelgrpc DoS vulnerability ([#5208](https://github.com/kedacore/keda/issues/5208))
 - **General**: Prevented memory leak generated by not correctly cleaning http connections ([#5248](https://github.com/kedacore/keda/issues/5248))
 - **General**: Prevented stuck status due to timeouts during scalers generation ([#5083](https://github.com/kedacore/keda/issues/5083))
+- **General**: ScaledObject Validating Webhook should support dry-run=server requests ([#5306](https://github.com/kedacore/keda/issues/5306))
 - **AWS Scalers**: Ensure session tokens are included when instantiating AWS credentials ([#5156](https://github.com/kedacore/keda/issues/5156))
 - **Azure Pipelines**: No more HTTP 400 errors produced by poolName with spaces ([#5107](https://github.com/kedacore/keda/issues/5107))
 - **GCP pubsub scaler**: Added `project_id` to filter for metrics queries ([#5256](https://github.com/kedacore/keda/issues/5256))

apis/keda/v1alpha1/scaledobject_webhook.go

@@ -53,22 +53,54 @@ func (so *ScaledObject) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	kc = mgr.GetClient()
 	restMapper = mgr.GetRESTMapper()
 	return ctrl.NewWebhookManagedBy(mgr).
+		WithValidator(&ScaledObjectCustomValidator{}).
 		For(so).
 		Complete()
 }
 
 // +kubebuilder:webhook:path=/validate-keda-sh-v1alpha1-scaledobject,mutating=false,failurePolicy=ignore,sideEffects=None,groups=keda.sh,resources=scaledobjects,verbs=create;update,versions=v1alpha1,name=vscaledobject.kb.io,admissionReviewVersions=v1
 
-var _ webhook.Validator = &ScaledObject{}
+// ScaledObjectCustomValidator is a custom validator for ScaledObject objects
+type ScaledObjectCustomValidator struct{}
+
+func (socv ScaledObjectCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) {
+	request, err := admission.RequestFromContext(ctx)
+	if err != nil {
+		return nil, err
+	}
+	so := obj.(*ScaledObject)
+	return so.ValidateCreate(request.DryRun)
+}
+
+func (socv ScaledObjectCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) {
+	request, err := admission.RequestFromContext(ctx)
+	if err != nil {
+		return nil, err
+	}
+	so := newObj.(*ScaledObject)
+	old := oldObj.(*ScaledObject)
+	return so.ValidateUpdate(old, request.DryRun)
+}
+
+func (socv ScaledObjectCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) {
+	request, err := admission.RequestFromContext(ctx)
+	if err != nil {
+		return nil, err
+	}
+	so := obj.(*ScaledObject)
+	return so.ValidateDelete(request.DryRun)
+}
+
+var _ webhook.CustomValidator = &ScaledObjectCustomValidator{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
-func (so *ScaledObject) ValidateCreate() (admission.Warnings, error) {
+func (so *ScaledObject) ValidateCreate(dryRun *bool) (admission.Warnings, error) {
 	val, _ := json.MarshalIndent(so, "", "  ")
 	scaledobjectlog.V(1).Info(fmt.Sprintf("validating scaledobject creation for %s", string(val)))
-	return validateWorkload(so, "create")
+	return validateWorkload(so, "create", *dryRun)
 }
 
-func (so *ScaledObject) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
+func (so *ScaledObject) ValidateUpdate(old runtime.Object, dryRun *bool) (admission.Warnings, error) {
 	val, _ := json.MarshalIndent(so, "", "  ")
 	scaledobjectlog.V(1).Info(fmt.Sprintf("validating scaledobject update for %s", string(val)))
 
@@ -77,10 +109,10 @@ func (so *ScaledObject) ValidateUpdate(old runtime.Object) (admission.Warnings,
 		return nil, nil
 	}
 
-	return validateWorkload(so, "update")
+	return validateWorkload(so, "update", *dryRun)
 }
 
-func (so *ScaledObject) ValidateDelete() (admission.Warnings, error) {
+func (so *ScaledObject) ValidateDelete(_ *bool) (admission.Warnings, error) {
 	return nil, nil
 }
 
@@ -95,10 +127,10 @@ func isRemovingFinalizer(so *ScaledObject, old runtime.Object) bool {
 	return len(so.ObjectMeta.Finalizers) == 0 && len(oldSo.ObjectMeta.Finalizers) == 1 && soSpecString == oldSoSpecString
 }
 
-func validateWorkload(so *ScaledObject, action string) (admission.Warnings, error) {
+func validateWorkload(so *ScaledObject, action string, dryRun bool) (admission.Warnings, error) {
 	metricscollector.RecordScaledObjectValidatingTotal(so.Namespace, action)
 
-	verifyFunctions := []func(*ScaledObject, string) error{
+	verifyFunctions := []func(*ScaledObject, string, bool) error{
 		verifyCPUMemoryScalers,
 		verifyTriggers,
 		verifyScaledObjects,
@@ -107,7 +139,7 @@ func validateWorkload(so *ScaledObject, action string) (admission.Warnings, erro
 	}
 
 	for i := range verifyFunctions {
-		err := verifyFunctions[i](so, action)
+		err := verifyFunctions[i](so, action, dryRun)
 		if err != nil {
 			return nil, err
 		}
@@ -117,7 +149,7 @@ func validateWorkload(so *ScaledObject, action string) (admission.Warnings, erro
 	return nil, nil
 }
 
-func verifyReplicaCount(incomingSo *ScaledObject, action string) error {
+func verifyReplicaCount(incomingSo *ScaledObject, action string, _ bool) error {
 	err := CheckReplicaCountBoundsAreValid(incomingSo)
 	if err != nil {
 		scaledobjectlog.WithValues("name", incomingSo.Name).Error(err, "validation error")
@@ -126,7 +158,7 @@ func verifyReplicaCount(incomingSo *ScaledObject, action string) error {
 	return nil
 }
 
-func verifyTriggers(incomingSo *ScaledObject, action string) error {
+func verifyTriggers(incomingSo *ScaledObject, action string, _ bool) error {
 	err := ValidateTriggers(incomingSo.Spec.Triggers)
 	if err != nil {
 		scaledobjectlog.WithValues("name", incomingSo.Name).Error(err, "validation error")
@@ -135,7 +167,7 @@ func verifyTriggers(incomingSo *ScaledObject, action string) error {
 	return err
 }
 
-func verifyHpas(incomingSo *ScaledObject, action string) error {
+func verifyHpas(incomingSo *ScaledObject, action string, _ bool) error {
 	hpaList := &autoscalingv2.HorizontalPodAutoscalerList{}
 	opt := &client.ListOptions{
 		Namespace: incomingSo.Namespace,
@@ -190,7 +222,7 @@ func verifyHpas(incomingSo *ScaledObject, action string) error {
 	return nil
 }
 
-func verifyScaledObjects(incomingSo *ScaledObject, action string) error {
+func verifyScaledObjects(incomingSo *ScaledObject, action string, _ bool) error {
 	soList := &ScaledObjectList{}
 	opt := &client.ListOptions{
 		Namespace: incomingSo.Namespace,
@@ -241,7 +273,11 @@ func verifyScaledObjects(incomingSo *ScaledObject, action string) error {
 	return nil
 }
 
-func verifyCPUMemoryScalers(incomingSo *ScaledObject, action string) error {
+func verifyCPUMemoryScalers(incomingSo *ScaledObject, action string, dryRun bool) error {
+	if dryRun {
+		return nil
+	}
+
 	var podSpec *corev1.PodSpec
 	for _, trigger := range incomingSo.Spec.Triggers {
 		if trigger.Type == cpuString || trigger.Type == memoryString {

apis/keda/v1alpha1/scaledobject_webhook_test.go

@@ -29,6 +29,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 var _ = It("should validate the so creation when there isn't any hpa", func() {
@@ -232,6 +233,34 @@ var _ = It("should validate the so creation with cpu and memory when deployment
 	}).ShouldNot(HaveOccurred())
 })
 
+var _ = It("shouldn't validate the creation with cpu and memory when deployment is missing", func() {
+
+	namespaceName := "deployment-missing"
+	namespace := createNamespace(namespaceName)
+	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", true, map[string]string{}, "")
+
+	err := k8sClient.Create(context.Background(), namespace)
+	Expect(err).ToNot(HaveOccurred())
+
+	Eventually(func() error {
+		return k8sClient.Create(context.Background(), so)
+	}).Should(HaveOccurred())
+})
+
+var _ = It("should validate the creation with cpu and memory when deployment is missing and dry-run is true", func() {
+
+	namespaceName := "deployment-missing-dry-run"
+	namespace := createNamespace(namespaceName)
+	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", true, map[string]string{}, "")
+
+	err := k8sClient.Create(context.Background(), namespace)
+	Expect(err).ToNot(HaveOccurred())
+
+	Eventually(func() error {
+		return k8sClient.Create(context.Background(), so, client.DryRunAll)
+	}).ShouldNot(HaveOccurred())
+})
+
 var _ = It("shouldn't validate the so creation with cpu and memory when deployment hasn't got memory request", func() {
 
 	namespaceName := "deployment-no-memory-request"