Skip to content

Commit f42c1f1

Browse files
Fix transfer-hpa-ownership panic when hpa name not provided (kedacore#7260)
* chore: renormalize line endings Signed-off-by: James Williams <jamesleighwilliams@gmail.com> * fix: nil pointer when transfer-hpa-ownership is true but hpa name not specified (kedacore#7254) Signed-off-by: James Williams <jamesleighwilliams@gmail.com> * update changelog Signed-off-by: James Williams <jamesleighwilliams@gmail.com> * revert vendor changes Signed-off-by: James Williams <jamesleighwilliams@gmail.com> --------- Signed-off-by: James Williams <jamesleighwilliams@gmail.com>
1 parent cd4f7d7 commit f42c1f1

3 files changed

Lines changed: 32 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio
7777

7878
### Fixes
7979

80+
- **General**: Fix nil reference panic when transfer-hpa-ownership is set but no hpa name is provided ([#7254](https://github.com/kedacore/keda/issues/7254))
8081
- **General**: Fix race condition in paused-replicas annotation causing ScaledObject to get stuck ([#7231](https://github.com/kedacore/keda/issues/7231))
8182
- **General**: Use TriggerError when all ScaledJob triggers fail ([#7205](https://github.com/kedacore/keda/pull/7205))
8283
- **ActiveMQ Scaler**: Correct parse error ActiveMQ ([#7245](https://github.com/kedacore/keda/pull/7245))

apis/keda/v1alpha1/scaledobject_webhook.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,9 +277,15 @@ func verifyHpas(incomingSo *ScaledObject, action string, _ bool) error {
277277
}
278278

279279
if !owned {
280-
if incomingSo.Annotations[ScaledObjectTransferHpaOwnershipAnnotation] == "true" &&
281-
incomingSo.Spec.Advanced.HorizontalPodAutoscalerConfig.Name == hpa.Name {
282-
scaledobjectlog.Info(fmt.Sprintf("%s hpa ownership being transferred to %s", hpa.Name, incomingSo.Name))
280+
if incomingSo.Annotations[ScaledObjectTransferHpaOwnershipAnnotation] == "true" {
281+
if incomingSo.Spec.Advanced != nil && incomingSo.Spec.Advanced.HorizontalPodAutoscalerConfig != nil && incomingSo.Spec.Advanced.HorizontalPodAutoscalerConfig.Name == hpa.Name {
282+
scaledobjectlog.Info(fmt.Sprintf("%s hpa ownership being transferred to %s", hpa.Name, incomingSo.Name))
283+
} else {
284+
err = fmt.Errorf("the existing hpa '%s' for workload '%s' of type '%s' must be specified by name in advanced settings to enable ownership transfer", hpa.Name, incomingSo.Spec.ScaleTargetRef.Name, incomingSoGvkr.GVKString())
285+
scaledobjectlog.Error(err, "validation error")
286+
metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "transfer-ownership-missing-hpa-name")
287+
return err
288+
}
283289
} else {
284290
err = fmt.Errorf("the workload '%s' of type '%s' is already managed by the hpa '%s'", incomingSo.Spec.ScaleTargetRef.Name, incomingSoGvkr.GVKString(), hpa.Name)
285291
scaledobjectlog.Error(err, "validation error")

apis/keda/v1alpha1/scaledobject_webhook_test.go

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,10 +290,10 @@ var _ = It("should validate the so creation when the fallback is configured, and
290290
}).Should(HaveOccurred())
291291
})
292292

293-
var _ = It("shouldn't validate the so creation when there is another unmanaged hpa and so has transfer-hpa-ownership activated", func() {
293+
var _ = It("should validate the so creation when there is another unmanaged hpa and so has transfer-hpa-ownership activated and the name is specified", func() {
294294

295-
hpaName := "test-unmanaged-hpa-ownership"
296-
namespaceName := "unmanaged-hpa-ownership"
295+
hpaName := "test-unmanaged-hpa-ownership-with-name"
296+
namespaceName := "unmanaged-hpa-ownership-with-name"
297297
namespace := createNamespace(namespaceName)
298298
hpa := createHpa(hpaName, namespaceName, workloadName, "apps/v1", "Deployment", nil)
299299
so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{ScaledObjectTransferHpaOwnershipAnnotation: "true"}, hpaName)
@@ -309,6 +309,25 @@ var _ = It("shouldn't validate the so creation when there is another unmanaged h
309309
}).ShouldNot(HaveOccurred())
310310
})
311311

312+
var _ = It("shouldn't validate the so creation when there is another unmanaged hpa and so has transfer-hpa-ownership activated but no name specified", func() {
313+
314+
hpaName := "test-unmanaged-hpa-ownership-without-name"
315+
namespaceName := "unmanaged-hpa-ownership-without-name"
316+
namespace := createNamespace(namespaceName)
317+
hpa := createHpa(hpaName, namespaceName, workloadName, "apps/v1", "Deployment", nil)
318+
so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{ScaledObjectTransferHpaOwnershipAnnotation: "true"}, "")
319+
320+
err := k8sClient.Create(context.Background(), namespace)
321+
Expect(err).ToNot(HaveOccurred())
322+
323+
err = k8sClient.Create(context.Background(), hpa)
324+
Expect(err).ToNot(HaveOccurred())
325+
326+
Eventually(func() error {
327+
return k8sClient.Create(context.Background(), so)
328+
}).Should(HaveOccurred())
329+
})
330+
312331
var _ = It("shouldn't validate the so creation when hpa has shared-ownership unactivated", func() {
313332

314333
hpaName := "test-hpa-disabled-validation-by-hpa-ownership"

0 commit comments

Comments
 (0)