From ca8dd42efb6b53f8041aec6b54d4e3c7e5bc9368 Mon Sep 17 00:00:00 2001 From: Vivian Dai Date: Mon, 15 Jul 2024 09:30:49 +0900 Subject: [PATCH 1/3] Fix admission webhook validation not denying invalid fallbacks and replica counts Signed-off-by: Vivian Dai --- CHANGELOG.md | 1 + apis/keda/v1alpha1/scaledobject_types.go | 2 +- apis/keda/v1alpha1/scaledobject_webhook.go | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2790b3ae0ab..dcd684e209b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,7 @@ Here is an overview of all new **experimental** features: - **AWS Secret Manager**: Pod identity overrides are honored ([#6195](https://github.com/kedacore/keda/issues/6195)) - **Azure Event Hub Scaler**: Checkpointer errors are correctly handled ([#6084](https://github.com/kedacore/keda/issues/6084)) +- **General**: Fix admission webhook validation not denying invalid fallbacks and replica counts ([#5954](https://github.com/kedacore/keda/issues/5954)) - **Metrics API Scaler**: Prometheus metrics can have multiple labels ([#6077](https://github.com/kedacore/keda/issues/6077)) ### Deprecations diff --git a/apis/keda/v1alpha1/scaledobject_types.go b/apis/keda/v1alpha1/scaledobject_types.go index f73de8e0a69..52d72251573 100644 --- a/apis/keda/v1alpha1/scaledobject_types.go +++ b/apis/keda/v1alpha1/scaledobject_types.go @@ -284,7 +284,7 @@ func CheckFallbackValid(scaledObject *ScaledObject) error { for _, trigger := range scaledObject.Spec.Triggers { if trigger.Type == cpuString || trigger.Type == memoryString { - return fmt.Errorf("type is %s , but fallback it is not supported by the CPU & memory scalers", trigger.Type) + return fmt.Errorf("type is %s, but fallback it is not supported by the CPU & memory scalers", trigger.Type) } if trigger.MetricType != autoscalingv2.AverageValueMetricType { return fmt.Errorf("MetricType=%s, but Fallback can only be enabled for triggers with metric of type AverageValue", trigger.MetricType) diff --git a/apis/keda/v1alpha1/scaledobject_webhook.go b/apis/keda/v1alpha1/scaledobject_webhook.go index b3602d16739..b90eb851ea8 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook.go +++ b/apis/keda/v1alpha1/scaledobject_webhook.go @@ -165,6 +165,7 @@ func verifyReplicaCount(incomingSo *ScaledObject, action string, _ bool) error { if err != nil { scaledobjectlog.WithValues("name", incomingSo.Name).Error(err, "validation error") metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "incorrect-replicas") + return err } return nil } @@ -174,6 +175,7 @@ func verifyFallback(incomingSo *ScaledObject, action string, _ bool) error { if err != nil { scaledobjectlog.WithValues("name", incomingSo.Name).Error(err, "validation error") metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "incorrect-fallback") + return err } return nil } From 32ff7965e06c68d1fe6db9fb86ff064b9ecbde7e Mon Sep 17 00:00:00 2001 From: Vivian Dai Date: Tue, 13 Aug 2024 14:10:37 +0900 Subject: [PATCH 2/3] Fix tests for scaledobject webhook validating fallback and replica count Signed-off-by: Vivian Dai --- apis/keda/v1alpha1/scaledobject_webhook_test.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/apis/keda/v1alpha1/scaledobject_webhook_test.go b/apis/keda/v1alpha1/scaledobject_webhook_test.go index e30404d16c4..75604e6f3c5 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook_test.go +++ b/apis/keda/v1alpha1/scaledobject_webhook_test.go @@ -153,9 +153,8 @@ var _ = It("shouldn't validate the so creation when the replica counts are wrong err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) - Eventually(func() error { - return k8sClient.Create(context.Background(), so) - }).Should(HaveOccurred()) + err = k8sClient.Create(context.Background(), so) + Expect(err).To(HaveOccurred()) }) var _ = It("shouldn't validate the so creation when the fallback is wrong", func() { @@ -171,9 +170,8 @@ var _ = It("shouldn't validate the so creation when the fallback is wrong", func err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) - Eventually(func() error { - return k8sClient.Create(context.Background(), so) - }).Should(HaveOccurred()) + err = k8sClient.Create(context.Background(), so) + Expect(err).To(HaveOccurred()) }) var _ = It("shouldn't validate the so creation When the fallback are configured and the scaler is either CPU or memory.", func() { @@ -191,9 +189,8 @@ var _ = It("shouldn't validate the so creation When the fallback are configured err = k8sClient.Create(context.Background(), workload) Expect(err).ToNot(HaveOccurred()) - Eventually(func() error { - return k8sClient.Create(context.Background(), so) - }).Should(HaveOccurred()) + err = k8sClient.Create(context.Background(), so) + Expect(err).To(HaveOccurred()) }) var _ = It("shouldn't validate the so creation when there is another unmanaged hpa and so has transfer-hpa-ownership activated", func() { From 10b37cc1a5f668d78c9d1ef866858d65a1efcaac Mon Sep 17 00:00:00 2001 From: Vivian Dai Date: Mon, 7 Oct 2024 21:48:42 +0900 Subject: [PATCH 3/3] Fix changelog sort order --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dcd684e209b..6029fc8933b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,9 +80,9 @@ Here is an overview of all new **experimental** features: ### Fixes +- **General**: Fix admission webhook validation not denying invalid fallbacks and replica counts ([#5954](https://github.com/kedacore/keda/issues/5954)) - **AWS Secret Manager**: Pod identity overrides are honored ([#6195](https://github.com/kedacore/keda/issues/6195)) - **Azure Event Hub Scaler**: Checkpointer errors are correctly handled ([#6084](https://github.com/kedacore/keda/issues/6084)) -- **General**: Fix admission webhook validation not denying invalid fallbacks and replica counts ([#5954](https://github.com/kedacore/keda/issues/5954)) - **Metrics API Scaler**: Prometheus metrics can have multiple labels ([#6077](https://github.com/kedacore/keda/issues/6077)) ### Deprecations