feat: add scaletargetref exists check in webhook and revise some variable name

CHANGELOG.md

@@ -58,6 +58,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio
 
 ### New
 
+- **General**: Add scaleTargetRef exists check in webhook and revise some variable name ([#6350](https://github.com/kedacore/keda/pull/6350))
 - **General**: Enable OpenSSF Scorecard to enhance security practices across the project ([#5913](https://github.com/kedacore/keda/issues/5913))
 - **General**: Introduce new NSQ scaler ([#3281](https://github.com/kedacore/keda/issues/3281))
 

apis/keda/v1alpha1/scaledobject_types.go

@@ -17,12 +17,18 @@ limitations under the License.
 package v1alpha1
 
 import (
+	"context"
 	"fmt"
 	"reflect"
 	"strconv"
 
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/kedacore/keda/v2/pkg/common/message"
 )
 
 // +genclient
@@ -254,6 +260,25 @@ func (so *ScaledObject) GetHPAMaxReplicas() int32 {
 	return defaultHPAMaxReplicas
 }
 
+// CheckScaleTargetRefIfExist checks if scaleTargetRef of ScaledObject exists
+func (so *ScaledObject) CheckScaleTargetRefIfExist(ctx context.Context, restMapper meta.RESTMapper, getFromCacheOrDirect func(ctx context.Context, key client.ObjectKey, obj client.Object) error) error {
+	soGvkr, err := ParseGVKR(restMapper, so.Spec.ScaleTargetRef.APIVersion, so.Spec.ScaleTargetRef.Kind)
+	if err != nil {
+		msg := "Failed to parse Group, Version, Kind, Resource"
+		scaledobjectlog.Error(err, msg, "apiVersion", so.Spec.ScaleTargetRef.APIVersion, "kind", so.Spec.ScaleTargetRef.Kind)
+		return err
+	}
+	gvkString := soGvkr.GVKString()
+	unstruct := &unstructured.Unstructured{}
+	unstruct.SetGroupVersionKind(soGvkr.GroupVersionKind())
+	if err := getFromCacheOrDirect(ctx, client.ObjectKey{Namespace: so.Namespace, Name: so.Spec.ScaleTargetRef.Name}, unstruct); err != nil {
+		// resource doesn't exist
+		scaledobjectlog.Error(err, message.ScaleTargetNotFoundMsg, "resource", gvkString, "name", so.Spec.ScaleTargetRef.Name)
+		return err
+	}
+	return nil
+}
+
 // checkReplicaCountBoundsAreValid checks that Idle/Min/Max ReplicaCount defined in ScaledObject are correctly specified
 // i.e. that Min is not greater than Max or Idle greater or equal to Min
 func CheckReplicaCountBoundsAreValid(scaledObject *ScaledObject) error {

apis/keda/v1alpha1/scaledobject_webhook.go

@@ -288,8 +288,11 @@ func verifyScaledObjects(incomingSo *ScaledObject, action string, _ bool) error
 	if err != nil {
 		return err
 	}
+	if err := incomingSo.CheckScaleTargetRefIfExist(context.Background(), restMapper, getFromCacheOrDirect); err != nil {
+		return err
+	}
 
-	incomingSoGckr, err := ParseGVKR(restMapper, incomingSo.Spec.ScaleTargetRef.APIVersion, incomingSo.Spec.ScaleTargetRef.Kind)
+	incomingSoGvkr, err := ParseGVKR(restMapper, incomingSo.Spec.ScaleTargetRef.APIVersion, incomingSo.Spec.ScaleTargetRef.Kind)
 	if err != nil {
 		scaledobjectlog.Error(err, "Failed to parse Group, Version, Kind, Resource from incoming ScaledObject", "apiVersion", incomingSo.Spec.ScaleTargetRef.APIVersion, "kind", incomingSo.Spec.ScaleTargetRef.Kind)
 		return err
@@ -303,15 +306,15 @@ func verifyScaledObjects(incomingSo *ScaledObject, action string, _ bool) error
 		val, _ := json.MarshalIndent(so, "", "  ")
 		scaledobjectlog.V(1).Info(fmt.Sprintf("checking scaledobject %s: %v", so.Name, string(val)))
 
-		soGckr, err := ParseGVKR(restMapper, so.Spec.ScaleTargetRef.APIVersion, so.Spec.ScaleTargetRef.Kind)
+		soGvkr, err := ParseGVKR(restMapper, so.Spec.ScaleTargetRef.APIVersion, so.Spec.ScaleTargetRef.Kind)
 		if err != nil {
 			scaledobjectlog.Error(err, "Failed to parse Group, Version, Kind, Resource from ScaledObject", "soName", so.Name, "apiVersion", so.Spec.ScaleTargetRef.APIVersion, "kind", so.Spec.ScaleTargetRef.Kind)
 			return err
 		}
 
-		if soGckr.GVKString() == incomingSoGckr.GVKString() &&
+		if soGvkr.GVKString() == incomingSoGvkr.GVKString() &&
 			so.Spec.ScaleTargetRef.Name == incomingSo.Spec.ScaleTargetRef.Name {
-			err = fmt.Errorf("the workload '%s' of type '%s' is already managed by the ScaledObject '%s'", so.Spec.ScaleTargetRef.Name, incomingSoGckr.GVKString(), so.Name)
+			err = fmt.Errorf("the workload '%s' of type '%s' is already managed by the ScaledObject '%s'", so.Spec.ScaleTargetRef.Name, incomingSoGvkr.GVKString(), so.Name)
 			scaledobjectlog.Error(err, "validation error")
 			metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "other-scaled-object")
 			return err

apis/keda/v1alpha1/scaledobject_webhook_test.go

@@ -36,11 +36,17 @@ var _ = It("should validate the so creation when there isn't any hpa", func() {
 
 	namespaceName := "valid"
 	namespace := createNamespace(namespaceName)
+
+	workload := createDeployment(namespaceName, false, false)
+
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
 
 	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())
@@ -50,12 +56,24 @@ var _ = It("should validate the so creation when there are other SO for other wo
 
 	namespaceName := "valid-multiple-so"
 	namespace := createNamespace(namespaceName)
+
+	workload := createDeployment(namespaceName, false, false)
+	otherWorkload := createDeployment(namespaceName, false, false)
+	otherWorkload.Name = "other-workload"
+	otherWorkload.Spec.Template.Name = "other-workload"
+
 	so1 := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
 	so2 := createScaledObject("other-so-name", namespaceName, "other-workload", "apps/v1", "Deployment", false, map[string]string{}, "")
 
 	err := k8sClient.Create(context.Background(), namespace)
 	Expect(err).ToNot(HaveOccurred())
 
+	err = k8sClient.Create(context.Background(), workload)
+	Expect(err).ToNot(HaveOccurred())
+
+	err = k8sClient.Create(context.Background(), otherWorkload)
+	Expect(err).ToNot(HaveOccurred())
+
 	err = k8sClient.Create(context.Background(), so1)
 	Expect(err).ToNot(HaveOccurred())
 
@@ -68,12 +86,18 @@ var _ = It("should validate the so creation when there are other HPA for other w
 
 	namespaceName := "valid-other-hpa"
 	namespace := createNamespace(namespaceName)
+
+	workload := createDeployment(namespaceName, false, false)
+
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
 	hpa := createHpa("other-hpa-name", namespaceName, "other-workload", "apps/v1", "Deployment", nil)
 
 	err := k8sClient.Create(context.Background(), namespace)
 	Expect(err).ToNot(HaveOccurred())
 
+	err = k8sClient.Create(context.Background(), workload)
+	Expect(err).ToNot(HaveOccurred())
+
 	err = k8sClient.Create(context.Background(), hpa)
 	Expect(err).ToNot(HaveOccurred())
 
@@ -87,12 +111,18 @@ var _ = It("should validate the so creation when it's own hpa is already generat
 	hpaName := "test-so-hpa"
 	namespaceName := "own-hpa"
 	namespace := createNamespace(namespaceName)
+
+	workload := createDeployment(namespaceName, false, false)
+
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
 	hpa := createHpa(hpaName, namespaceName, workloadName, "apps/v1", "Deployment", so)
 
 	err := k8sClient.Create(context.Background(), namespace)
 	Expect(err).ToNot(HaveOccurred())
 
+	err = k8sClient.Create(context.Background(), workload)
+	Expect(err).ToNot(HaveOccurred())
+
 	err = k8sClient.Create(context.Background(), hpa)
 	Expect(err).ToNot(HaveOccurred())
 
@@ -106,12 +136,18 @@ var _ = It("should validate the so update when it's own hpa is already generated
 	hpaName := "test-so-hpa"
 	namespaceName := "update-so"
 	namespace := createNamespace(namespaceName)
+
+	workload := createDeployment(namespaceName, false, false)
+
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
 	hpa := createHpa(hpaName, namespaceName, workloadName, "apps/v1", "Deployment", so)
 
 	err := k8sClient.Create(context.Background(), namespace)
 	Expect(err).ToNot(HaveOccurred())
 
+	err = k8sClient.Create(context.Background(), workload)
+	Expect(err).ToNot(HaveOccurred())
+
 	err = k8sClient.Create(context.Background(), hpa)
 	Expect(err).ToNot(HaveOccurred())
 
@@ -129,12 +165,18 @@ var _ = It("shouldn't validate the so creation when there is another unmanaged h
 	hpaName := "test-unmanaged-hpa"
 	namespaceName := "unmanaged-hpa"
 	namespace := createNamespace(namespaceName)
+
+	workload := createDeployment(namespaceName, false, false)
+
 	hpa := createHpa(hpaName, namespaceName, workloadName, "apps/v1", "Deployment", nil)
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
 
 	err := k8sClient.Create(context.Background(), namespace)
 	Expect(err).ToNot(HaveOccurred())
 
+	err = k8sClient.Create(context.Background(), workload)
+	Expect(err).ToNot(HaveOccurred())
+
 	err = k8sClient.Create(context.Background(), hpa)
 	Expect(err).ToNot(HaveOccurred())
 
@@ -146,13 +188,19 @@ var _ = It("shouldn't validate the so creation when there is another unmanaged h
 var _ = It("shouldn't validate the so creation when the replica counts are wrong", func() {
 	namespaceName := "wrong-replica-count"
 	namespace := createNamespace(namespaceName)
+
+	workload := createDeployment(namespaceName, false, false)
+
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
 	so.Spec.MinReplicaCount = ptr.To[int32](10)
 	so.Spec.MaxReplicaCount = ptr.To[int32](5)
 
 	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())
@@ -162,6 +210,8 @@ var _ = It("shouldn't validate the so creation when the fallback is wrong", func
 	namespaceName := "wrong-fallback"
 	namespace := createNamespace(namespaceName)
 
+	workload := createDeployment(namespaceName, false, false)
+
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
 	so.Spec.Fallback = &Fallback{
 		FailureThreshold: -1,
@@ -171,6 +221,9 @@ var _ = It("shouldn't validate the so creation when the fallback is wrong", func
 	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())
@@ -201,12 +254,18 @@ var _ = It("shouldn't validate the so creation when there is another unmanaged h
 	hpaName := "test-unmanaged-hpa-ownership"
 	namespaceName := "unmanaged-hpa-ownership"
 	namespace := createNamespace(namespaceName)
+
+	workload := createDeployment(namespaceName, false, false)
+
 	hpa := createHpa(hpaName, namespaceName, workloadName, "apps/v1", "Deployment", nil)
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{ScaledObjectTransferHpaOwnershipAnnotation: "true"}, hpaName)
 
 	err := k8sClient.Create(context.Background(), namespace)
 	Expect(err).ToNot(HaveOccurred())
 
+	err = k8sClient.Create(context.Background(), workload)
+	Expect(err).ToNot(HaveOccurred())
+
 	err = k8sClient.Create(context.Background(), hpa)
 	Expect(err).ToNot(HaveOccurred())
 
@@ -220,13 +279,19 @@ var _ = It("shouldn't validate the so creation when hpa has shared-ownership una
 	hpaName := "test-hpa-disabled-validation-by-hpa-ownership"
 	namespaceName := "hpa-ownership"
 	namespace := createNamespace(namespaceName)
+
+	workload := createDeployment(namespaceName, false, false)
+
 	hpa := createHpa(hpaName, namespaceName, workloadName, "apps/v1", "Deployment", nil)
 	hpa.ObjectMeta.Annotations = map[string]string{ValidationsHpaOwnershipAnnotation: "false"}
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{ScaledObjectTransferHpaOwnershipAnnotation: "false"}, hpaName)
 
 	err := k8sClient.Create(context.Background(), namespace)
 	Expect(err).ToNot(HaveOccurred())
 
+	err = k8sClient.Create(context.Background(), workload)
+	Expect(err).ToNot(HaveOccurred())
+
 	err = k8sClient.Create(context.Background(), hpa)
 	Expect(err).ToNot(HaveOccurred())
 
@@ -240,12 +305,18 @@ var _ = It("shouldn't validate the so creation when there is another so", func()
 	so2Name := "test-so2"
 	namespaceName := "managed-hpa"
 	namespace := createNamespace(namespaceName)
+
+	workload := createDeployment(namespaceName, false, false)
+
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
 	so2 := createScaledObject(so2Name, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
 
 	err := k8sClient.Create(context.Background(), namespace)
 	Expect(err).ToNot(HaveOccurred())
 
+	err = k8sClient.Create(context.Background(), workload)
+	Expect(err).ToNot(HaveOccurred())
+
 	err = k8sClient.Create(context.Background(), so2)
 	Expect(err).ToNot(HaveOccurred())
 
@@ -291,29 +362,51 @@ 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() {
+var _ = It("shouldn't validate the so creation when deployment is missing", func() {
 
 	namespaceName := "deployment-missing"
 	namespace := createNamespace(namespaceName)
+	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
+
+	err := k8sClient.Create(context.Background(), namespace)
+	Expect(err).ToNot(HaveOccurred())
+
+	Eventually(func() error {
+		return k8sClient.Create(context.Background(), so, client.DryRunAll)
+	}).Should(HaveOccurred())
+})
+
+var _ = It("should validate the so creation with cpu and memory", func() {
+
+	namespaceName := "deployment-not-missing"
+	namespace := createNamespace(namespaceName)
+	workload := createDeployment(namespaceName, true, true)
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", true, map[string]string{}, "")
 
 	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())
+	}).ShouldNot(HaveOccurred())
 })
 
-var _ = It("should validate the creation with cpu and memory when deployment is missing and dry-run is true", func() {
+var _ = It("should validate the so creation with cpu and memory when dry-run is true", func() {
 
-	namespaceName := "deployment-missing-dry-run"
+	namespaceName := "deployment-dry-run"
 	namespace := createNamespace(namespaceName)
+	workload := createDeployment(namespaceName, true, true)
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", true, map[string]string{}, "")
 
 	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, client.DryRunAll)
 	}).ShouldNot(HaveOccurred())
@@ -464,7 +557,7 @@ var _ = It("shouldn't validate the so creation with cpu and memory when stateful
 	}).Should(HaveOccurred())
 })
 
-var _ = It("should validate the so creation without cpu and memory when custom resources", func() {
+var _ = It("shouldn't validate the so creation without cpu and memory when custom resources and real StatefulSet not found", func() {
 
 	namespaceName := "crd-not-resources"
 	namespace := createNamespace(namespaceName)
@@ -475,7 +568,7 @@ var _ = It("should validate the so creation without cpu and memory when custom r
 
 	Eventually(func() error {
 		return k8sClient.Create(context.Background(), so)
-	}).ShouldNot(HaveOccurred())
+	}).Should(HaveOccurred())
 })
 
 var _ = It("should validate so creation when all requirements are met for scaling to zero with cpu scaler", func() {
@@ -623,6 +716,7 @@ var _ = It("shouldn't create so when stabilizationWindowSeconds exceeds 3600", f
 
 	namespaceName := "fail-so-creation"
 	namespace := createNamespace(namespaceName)
+	workload := createDeployment(namespaceName, false, false)
 	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
 	so.Spec.Advanced.HorizontalPodAutoscalerConfig = &HorizontalPodAutoscalerConfig{
 		Behavior: &v2.HorizontalPodAutoscalerBehavior{
@@ -634,6 +728,9 @@ var _ = It("shouldn't create so when stabilizationWindowSeconds exceeds 3600", f
 	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)
 	}).