diff --git a/BUILD.md b/BUILD.md index e417655c7a6..28f798891d8 100644 --- a/BUILD.md +++ b/BUILD.md @@ -287,7 +287,8 @@ Follow these instructions if you want to debug the KEDA webhook using VS Code. clientConfig: url: "https://${YOUR_URL}/validate-keda-sh-v1alpha1-scaledobject" ``` - **Note:** You could need to define also the key `caBundle` with the CA bundle encoded in base64 if the cluster can get it during the manifest apply (this happens with localtunnel for instance) + **Note:** You need to define also the key `caBundle` with the CA bundle encoded in base64. This `caBundle` is the pem file from the CA used to sign the certificate. Remember to disable the `caBundle` inyection to avoid unintended rewrites of your `caBundle` (by KEDA operator or by any other 3rd party) + 4. Deploy CRDs and KEDA into `keda` namespace ```bash diff --git a/CHANGELOG.md b/CHANGELOG.md index cece6e29b2c..3a584c5608e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio ### Fixes +- **Admission Webhooks**: Allow to remove the finalizer even if the ScaledObject isn't valid ([#4396](https://github.com/kedacore/keda/issue/4396)) - **AWS SQS Scaler**: Respect `scaleOnInFlight` value ([#4276](https://github.com/kedacore/keda/issue/4276)) - **Azure Pipelines**: Fix for disallowing `$top` on query when using `meta.parentID` method ([#4397]) - **Azure Pipelines**: Respect all required demands ([#4404](https://github.com/kedacore/keda/issues/4404)) diff --git a/apis/keda/v1alpha1/scaledobject_webhook.go b/apis/keda/v1alpha1/scaledobject_webhook.go index 3f6177db06a..a811b01411f 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook.go +++ b/apis/keda/v1alpha1/scaledobject_webhook.go @@ -62,6 +62,12 @@ func (so *ScaledObject) ValidateCreate() error { func (so *ScaledObject) ValidateUpdate(old runtime.Object) error { val, _ := json.MarshalIndent(so, "", " ") scaledobjectlog.V(1).Info(fmt.Sprintf("validating scaledobject update for %s", string(val))) + + if isRemovingFinalizer(so, old) { + scaledobjectlog.V(1).Info("finalizer removal, skipping validation") + return nil + } + return validateWorkload(so, "update") } @@ -69,6 +75,17 @@ func (so *ScaledObject) ValidateDelete() error { return nil } +func isRemovingFinalizer(so *ScaledObject, old runtime.Object) bool { + oldSo := old.(*ScaledObject) + + soSpec, _ := json.MarshalIndent(so.Spec, "", " ") + oldSoSpec, _ := json.MarshalIndent(oldSo.Spec, "", " ") + soSpecString := string(soSpec) + oldSoSpecString := string(oldSoSpec) + + return len(so.ObjectMeta.Finalizers) == 0 && len(oldSo.ObjectMeta.Finalizers) == 1 && soSpecString == oldSoSpecString +} + func validateWorkload(so *ScaledObject, action string) error { prommetrics.RecordScaledObjectValidatingTotal(so.Namespace, action) err := verifyCPUMemoryScalers(so, action) diff --git a/apis/keda/v1alpha1/scaledobject_webhook_test.go b/apis/keda/v1alpha1/scaledobject_webhook_test.go index 9d2ad532f3b..7113dadf0f1 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook_test.go +++ b/apis/keda/v1alpha1/scaledobject_webhook_test.go @@ -154,8 +154,9 @@ var _ = It("should validate the so creation when there isn't any hpa", func() { err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) - Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).ShouldNot(HaveOccurred()) }) var _ = It("should validate the so creation when there are other SO for other workloads", func() { @@ -171,8 +172,9 @@ var _ = It("should validate the so creation when there are other SO for other wo err = k8sClient.Create(context.Background(), so1) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so2) - Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so2) + }).ShouldNot(HaveOccurred()) }) var _ = It("should validate the so creation when there are other HPA for other workloads", func() { @@ -188,8 +190,9 @@ var _ = It("should validate the so creation when there are other HPA for other w err = k8sClient.Create(context.Background(), hpa) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) - Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).ShouldNot(HaveOccurred()) }) var _ = It("should validate the so creation when it's own hpa is already generated", func() { @@ -206,8 +209,9 @@ var _ = It("should validate the so creation when it's own hpa is already generat err = k8sClient.Create(context.Background(), hpa) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) - Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).ShouldNot(HaveOccurred()) }) var _ = It("should validate the so update when it's own hpa is already generated", func() { @@ -228,8 +232,9 @@ var _ = It("should validate the so update when it's own hpa is already generated Expect(err).ToNot(HaveOccurred()) so.Spec.MaxReplicaCount = pointer.Int32(7) - err = k8sClient.Update(context.Background(), so) - Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { + return k8sClient.Update(context.Background(), so) + }).ShouldNot(HaveOccurred()) }) var _ = It("shouldn't validate the so creation when there is another unmanaged hpa", func() { @@ -246,8 +251,9 @@ var _ = It("shouldn't validate the so creation when there is another unmanaged h err = k8sClient.Create(context.Background(), hpa) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) - Expect(err).To(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).Should(HaveOccurred()) }) var _ = It("shouldn't validate the so creation when there is another so", func() { @@ -264,8 +270,9 @@ var _ = It("shouldn't validate the so creation when there is another so", func() err = k8sClient.Create(context.Background(), so2) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) - Expect(err).To(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).Should(HaveOccurred()) }) var _ = It("shouldn't validate the so creation when there is another hpa with custom apis", func() { @@ -282,8 +289,9 @@ var _ = It("shouldn't validate the so creation when there is another hpa with cu err = k8sClient.Create(context.Background(), hpa) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) - Expect(err).To(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).Should(HaveOccurred()) }) var _ = It("should validate the so creation with cpu and memory when deployment has requests", func() { @@ -299,8 +307,9 @@ var _ = It("should validate the so creation with cpu and memory when deployment err = k8sClient.Create(context.Background(), workload) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) - Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).ShouldNot(HaveOccurred()) }) var _ = It("shouldn't validate the so creation with cpu and memory when deployment hasn't got memory request", func() { @@ -316,8 +325,9 @@ var _ = It("shouldn't validate the so creation with cpu and memory when deployme err = k8sClient.Create(context.Background(), workload) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) - Expect(err).To(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).Should(HaveOccurred()) }) var _ = It("shouldn't validate the so creation with cpu and memory when deployment hasn't got cpu request", func() { @@ -333,8 +343,9 @@ var _ = It("shouldn't validate the so creation with cpu and memory when deployme err = k8sClient.Create(context.Background(), workload) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) - Expect(err).To(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).Should(HaveOccurred()) }) var _ = It("should validate the so creation with cpu and memory when statefulset has requests", func() { @@ -350,8 +361,9 @@ var _ = It("should validate the so creation with cpu and memory when statefulset err = k8sClient.Create(context.Background(), workload) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) - Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).ShouldNot(HaveOccurred()) }) var _ = It("shouldn't validate the so creation with cpu and memory when statefulset hasn't got memory request", func() { @@ -367,8 +379,9 @@ var _ = It("shouldn't validate the so creation with cpu and memory when stateful err = k8sClient.Create(context.Background(), workload) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) - Expect(err).To(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).Should(HaveOccurred()) }) var _ = It("shouldn't validate the so creation with cpu and memory when statefulset hasn't got cpu request", func() { @@ -384,8 +397,9 @@ var _ = It("shouldn't validate the so creation with cpu and memory when stateful err = k8sClient.Create(context.Background(), workload) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) - Expect(err).To(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).Should(HaveOccurred()) }) var _ = It("should validate the so creation without cpu and memory when custom resources", func() { @@ -397,8 +411,88 @@ var _ = It("should validate the so creation without cpu and memory when custom r err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), so) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).ShouldNot(HaveOccurred()) +}) + +var _ = It("should validate so creation when all requirements are met for scaling to zero with cpu scaler", func() { + namespaceName := "scale-to-zero-good" + namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, true, false) + + so := createScaledObjectSTZ(soName, namespaceName, workloadName, 0, 5, true) + + err := k8sClient.Create(context.Background(), namespace) + Expect(err).ToNot(HaveOccurred()) + + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).ShouldNot(HaveOccurred()) +}) + +var _ = It("shouldn't validate so creation with cpu scaler requirements not being met for scaling to 0", func() { + namespaceName := "scale-to-zero-min-replicas-bad" + namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, true, false) + + so := createScaledObjectSTZ(soName, namespaceName, workloadName, 0, 5, false) + + err := k8sClient.Create(context.Background(), namespace) + Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).Should(HaveOccurred()) +}) + +var _ = It("should validate so creation when min replicas is > 0 with only cpu scaler given", func() { + namespaceName := "scale-to-zero-no-external-trigger-good" + namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, true, false) + + so := createScaledObjectSTZ(soName, namespaceName, workloadName, 1, 5, false) + + err := k8sClient.Create(context.Background(), namespace) + Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).ShouldNot(HaveOccurred()) + +}) + +var _ = It("should validate the so update if it's removing the finalizer even if it's invalid", func() { + + namespaceName := "removing-finalizers" + namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, true, true) + so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", true) + so.ObjectMeta.Finalizers = append(so.ObjectMeta.Finalizers, "finalizer") + + err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).ShouldNot(HaveOccurred()) + + workload.Spec.Template.Spec.Containers[0].Resources.Requests = nil + err = k8sClient.Update(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + + so.ObjectMeta.Finalizers = []string{} + Eventually(func() error { + return k8sClient.Update(context.Background(), so) + }).ShouldNot(HaveOccurred()) }) var _ = AfterSuite(func() { @@ -595,3 +689,44 @@ func createStatefulSet(namespace string, hasCPU, hasMemory bool) *appsv1.Statefu }, } } + +func createScaledObjectSTZ(name string, namespace string, targetName string, minReplicas int32, maxReplicas int32, hasExternalTrigger bool) *ScaledObject { + triggers := []ScaleTriggers{ + { + Type: "cpu", + Metadata: map[string]string{ + "value": "10", + }, + }, + } + if hasExternalTrigger { + kubeWorkloadTrigger := ScaleTriggers{ + Type: "kubernetes-workload", + Metadata: map[string]string{ + "podSelector": "pod=workload-test", + "value": "1", + }, + } + triggers = append(triggers, kubeWorkloadTrigger) + } + return &ScaledObject{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + UID: types.UID(name), + }, + TypeMeta: metav1.TypeMeta{ + Kind: "ScaledObject", + APIVersion: "keda.sh", + }, + Spec: ScaledObjectSpec{ + ScaleTargetRef: &ScaleTarget{ + Name: targetName, + }, + MinReplicaCount: pointer.Int32(minReplicas), + MaxReplicaCount: pointer.Int32(maxReplicas), + CooldownPeriod: pointer.Int32(1), + Triggers: triggers, + }, + } +}