Skip to content

fix(webhook): use field indexes in verifyScaledObjects to avoid O(N) admission cost #7681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ggarb wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(webhook): use field indexes in verifyScaledObjects to avoid O(N) admission cost #7681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
966b7c5
fix(webhook): use field indexes in verifyScaledObjects to avoid O(N) …
ggarb Apr 23, 2026
6e80753
fix(webhook): add field index for verifyHpas to avoid O(N) HPA list
ggarb Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio

### Fixes

- **General**: Add cache field indexes to `verifyScaledObjects` so ScaledObject admission validates duplicate scaleTargetRef/HPA in O(1) instead of O(N), eliminating the webhook OOM under high-scale creation bursts ([#7681](https://github.com/kedacore/keda/pull/7681))
- **General**: Check updated status for Fallback condition instead of ScaledObject ([#7488](https://github.com/kedacore/keda/issues/7488))
- **General**: Fix int64 overflow in milli-quantity conversion for very large metric values ([#7441](https://github.com/kedacore/keda/issues/7441))
- **General**: Fix ScaledObject admission webhook to return validation error from `verifyReplicaCount`, preventing invalid ScaledObjects from being created ([#5954](https://github.com/kedacore/keda/issues/5954))
Expand Down
116 changes: 85 additions & 31 deletions apis/keda/v1alpha1/scaledobject_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,45 @@ var restMapper meta.RESTMapper
var memoryString = "memory"
var cpuString = "cpu"

// Field index names used by verifyScaledObjects to avoid O(N) full-namespace
// list scans on every SO admission. Without these indexes each admission must
// DeepCopy every ScaledObject in the namespace to find conflicts; at 60k SOs
// each admission allocates ~900 MiB, which is why the webhook OOMs under
// creation bursts. The indexes narrow candidates to 0–1 objects.
Comment on lines +55 to +59
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment claims the indexes narrow candidates to “0–1 objects” and describes the lookups as O(1). In practice the ScaledObject index is only on spec.scaleTargetRef.name, so a namespace can legitimately have multiple ScaledObjects targeting different GVKs with the same name (e.g., Deployment "foo" and StatefulSet "foo"), making the lookup O(k) where k is the number of matches for that name. Consider rewording this comment to avoid implying a strict O(1)/0–1 guarantee.

Copilot uses AI. Check for mistakes.
const (
scaleTargetRefNameIdx = "spec.scaleTargetRef.name"
hpaNameIdx = "spec.hpaName"
// hpaScaleTargetNameIdx indexes HPA objects by spec.scaleTargetRef.name so
// verifyHpas can issue an O(1) lookup instead of listing every HPA in the
// namespace. Index names are scoped per-GVK so reusing the same path string
// as scaleTargetRefNameIdx is safe.
hpaScaleTargetNameIdx = "spec.scaleTargetRef.name"
Comment on lines +61 to +67
Copy link
Copy Markdown
Member

@rickbrouwer rickbrouwer Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const scaleTargetRefNameIdx and hpaScaleTargetNameIdx has both the same value

)

func (so *ScaledObject) SetupWebhookWithManager(mgr ctrl.Manager, cacheMissFallback bool) error {
// Register field indexes before wiring the webhook so that
// verifyScaledObjects can issue O(1) indexed lookups instead of
// O(N) full-namespace list scans. See the constants above.
ctx := context.Background()
if err := mgr.GetFieldIndexer().IndexField(ctx, &ScaledObject{}, scaleTargetRefNameIdx,
func(obj client.Object) []string {
return []string{obj.(*ScaledObject).Spec.ScaleTargetRef.Name}
}); err != nil {
return fmt.Errorf("failed to register index %q: %w", scaleTargetRefNameIdx, err)
}
if err := mgr.GetFieldIndexer().IndexField(ctx, &ScaledObject{}, hpaNameIdx,
func(obj client.Object) []string {
return []string{getHpaName(*obj.(*ScaledObject))}
}); err != nil {
return fmt.Errorf("failed to register index %q: %w", hpaNameIdx, err)
}
if err := mgr.GetFieldIndexer().IndexField(ctx, &autoscalingv2.HorizontalPodAutoscaler{}, hpaScaleTargetNameIdx,
func(obj client.Object) []string {
return []string{obj.(*autoscalingv2.HorizontalPodAutoscaler).Spec.ScaleTargetRef.Name}
}); err != nil {
return fmt.Errorf("failed to register HPA index %q: %w", hpaScaleTargetNameIdx, err)
}

err := setupKubernetesClients(mgr, cacheMissFallback)
if err != nil {
return fmt.Errorf("failed to setup kubernetes clients: %w", err)
Expand Down Expand Up @@ -235,10 +273,12 @@ func verifyTriggers(incomingObject interface{}, action string, _ bool) error {

func verifyHpas(incomingSo *ScaledObject, action string, _ bool) error {
hpaList := &autoscalingv2.HorizontalPodAutoscalerList{}
opt := &client.ListOptions{
Namespace: incomingSo.Namespace,
}
err := kc.List(context.Background(), hpaList, opt)
// Use the hpaScaleTargetNameIdx field index to narrow candidates to HPAs
// that target the same workload name, avoiding an O(N) full-namespace scan.
err := kc.List(context.Background(), hpaList,
client.InNamespace(incomingSo.Namespace),
client.MatchingFields{hpaScaleTargetNameIdx: incomingSo.Spec.ScaleTargetRef.Name},
)
if err != nil {
return err
}
Expand All @@ -254,8 +294,7 @@ func verifyHpas(incomingSo *ScaledObject, action string, _ bool) error {
if hpa.Annotations[ValidationsHpaOwnershipAnnotation] == "false" {
continue
}
val, _ := json.MarshalIndent(hpa, "", " ")
scaledobjectlog.V(1).Info(fmt.Sprintf("checking hpa %s: %v", hpa.Name, string(val)))
scaledobjectlog.V(1).Info("checking hpa", "name", hpa.Name)

hpaGvkr, err := ParseGVKR(restMapper, hpa.Spec.ScaleTargetRef.APIVersion, hpa.Spec.ScaleTargetRef.Kind)
if err != nil {
Expand Down Expand Up @@ -298,49 +337,64 @@ func verifyHpas(incomingSo *ScaledObject, action string, _ bool) error {
}

func verifyScaledObjects(incomingSo *ScaledObject, action string, _ bool) error {
soList := &ScaledObjectList{}
opt := &client.ListOptions{
Namespace: incomingSo.Namespace,
}
err := kc.List(context.Background(), soList, opt)
if err != nil {
return err
}
ctx := context.Background()

// Check 1: no other SO in this namespace already manages the same workload.
// Use the scaleTargetRef.name index so only SOs targeting the same resource
Copy link
Copy Markdown
Contributor

@dttung2905 dttung2905 Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After listing only by scaleTargetRef.name, the loop still must filter by GVK. Can you add or point to a test that has two targets named foo (e.g. Deployment + StatefulSet) and proves no false positive?

// name are fetched — typically 0 or 1 objects instead of the entire namespace.
incomingSoGckr, 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)
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
}

incomingSoHpaName := getHpaName(*incomingSo)
for _, so := range soList.Items {
targetCandidates := &ScaledObjectList{}
if err := kc.List(ctx, targetCandidates,
client.InNamespace(incomingSo.Namespace),
client.MatchingFields{scaleTargetRefNameIdx: incomingSo.Spec.ScaleTargetRef.Name},
); err != nil {
return err
}
for _, so := range targetCandidates.Items {
if so.Name == incomingSo.Name {
continue
}
val, _ := json.MarshalIndent(so, "", " ")
scaledobjectlog.V(1).Info(fmt.Sprintf("checking scaledobject %s: %v", so.Name, string(val)))

scaledobjectlog.V(1).Info("checking scaledobject for duplicate scaleTarget",
"name", so.Name, "namespace", so.Namespace)
soGckr, 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)
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() &&
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)
if soGckr.GVKString() == incomingSoGckr.GVKString() {
err = fmt.Errorf("the workload '%s' of type '%s' is already managed by the ScaledObject '%s'",
so.Spec.ScaleTargetRef.Name, incomingSoGckr.GVKString(), so.Name)
scaledobjectlog.Error(err, "validation error")
metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "other-scaled-object")
return err
}
}

if getHpaName(so) == incomingSoHpaName {
err = fmt.Errorf("the HPA '%s' is already managed by the ScaledObject '%s'", so.Spec.Advanced.HorizontalPodAutoscalerConfig.Name, so.Name)
scaledobjectlog.Error(err, "validation error")
metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "other-scaled-object-hpa")
return err
// Check 2: no other SO already owns the same HPA name.
// Use the hpaName index for the same reason.
incomingSoHpaName := getHpaName(*incomingSo)
hpaCandidates := &ScaledObjectList{}
if err := kc.List(ctx, hpaCandidates,
client.InNamespace(incomingSo.Namespace),
client.MatchingFields{hpaNameIdx: incomingSoHpaName},
); err != nil {
return err
}
for _, so := range hpaCandidates.Items {
if so.Name == incomingSo.Name {
continue
}
err = fmt.Errorf("the HPA '%s' is already managed by the ScaledObject '%s'",
incomingSoHpaName, so.Name)
scaledobjectlog.Error(err, "validation error")
metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "other-scaled-object-hpa")
return err
}

// verify ScalingModifiers structure if defined in ScaledObject
Expand Down
Loading