Skip to content

fix: return validation error from verifyReplicaCount in ScaledObject webhook#7562

Merged
rickbrouwer merged 2 commits into
kedacore:mainfrom
rohansood10:fix/5954-webhook-validation-return
Mar 20, 2026
Merged

fix: return validation error from verifyReplicaCount in ScaledObject webhook#7562
rickbrouwer merged 2 commits into
kedacore:mainfrom
rohansood10:fix/5954-webhook-validation-return

Conversation

@rohansood10

@rohansood10 rohansood10 commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Summary

The verifyReplicaCount function in the ScaledObject admission webhook logged validation errors from CheckReplicaCountBoundsAreValid but always returned nil, allowing ScaledObjects with invalid replica count bounds (e.g. minReplicaCount > maxReplicaCount) to be created despite failing validation.

Changes

  • verifyReplicaCount(): Return err instead of nil so the admission webhook properly rejects ScaledObjects with invalid replica count configurations
  • Remove the //nolint:unparam directive since the error return is no longer always nil
  • Add changelog entry under Unreleased/Fixes

This makes verifyReplicaCount consistent with the other verify functions (verifyFallback, verifyTriggers, etc.) which already correctly return their errors.

Checklist

  • I have verified that my change is according to the deprecations & breaking changes policy
  • Tests already exist for this scenario (shouldn't validate the so creation when the replica counts are wrong)
  • Changelog has been updated and is aligned with our changelog requirements, only when the change impacts end users
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #5954

…webhook

The verifyReplicaCount function in the ScaledObject admission webhook
logged the validation error from CheckReplicaCountBoundsAreValid but
always returned nil, allowing ScaledObjects with invalid replica count
bounds (e.g. minReplicaCount > maxReplicaCount) to be created despite
failing validation.

This fix changes the function to return the error, consistent with
the other verify functions (verifyFallback, verifyTriggers, etc.).
The //nolint:unparam directive is also removed since the error return
is no longer always nil.

Fixes kedacore#5954

Signed-off-by: Rohan Sood <[email protected]>
@rohansood10 rohansood10 requested a review from a team as a code owner March 19, 2026 20:46
@github-actions

Copy link
Copy Markdown

Thank you for your contribution! 🙏

Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected.

While you are waiting, make sure to:

  • Add an entry in our changelog in alphabetical order and link related issue
  • Update the documentation, if needed
  • Add unit & e2e tests for your changes
  • GitHub checks are passing
  • Is the DCO check failing? Here is how you can fix DCO issues

Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient.

Learn more about our contribution guide.

@keda-automation keda-automation requested a review from a team March 19, 2026 20:47
The validate-changelog pre-commit hook requires entries within each
section to be sorted alphabetically. Move the kedacore#5954 entry to its
correct position among the General entries.

Signed-off-by: Rohan Sood <[email protected]>

@JorTurFer JorTurFer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice catch!

@JorTurFer

JorTurFer commented Mar 19, 2026

Copy link
Copy Markdown
Member

/run-e2e internal
Update: You can check the progress here

@snyk-io

snyk-io Bot commented Mar 19, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@rickbrouwer rickbrouwer added the Awaiting/2nd-approval This PR needs one more approval review label Mar 20, 2026
@rickbrouwer rickbrouwer merged commit 2cd2cdd into kedacore:main Mar 20, 2026
26 checks passed
@rickbrouwer rickbrouwer removed the Awaiting/2nd-approval This PR needs one more approval review label Mar 20, 2026
jansworld pushed a commit to jansworld/keda that referenced this pull request Apr 10, 2026
…webhook (kedacore#7562)

* fix: return validation error from verifyReplicaCount in ScaledObject webhook

The verifyReplicaCount function in the ScaledObject admission webhook
logged the validation error from CheckReplicaCountBoundsAreValid but
always returned nil, allowing ScaledObjects with invalid replica count
bounds (e.g. minReplicaCount > maxReplicaCount) to be created despite
failing validation.

This fix changes the function to return the error, consistent with
the other verify functions (verifyFallback, verifyTriggers, etc.).
The //nolint:unparam directive is also removed since the error return
is no longer always nil.

Fixes kedacore#5954

Signed-off-by: Rohan Sood <[email protected]>

* fix: sort CHANGELOG Fixes entries alphabetically

The validate-changelog pre-commit hook requires entries within each
section to be sorted alphabetically. Move the kedacore#5954 entry to its
correct position among the General entries.

Signed-off-by: Rohan Sood <[email protected]>

---------

Signed-off-by: Rohan Sood <[email protected]>
Signed-off-by: jansworld <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ScaledObject still gets applied after failing validation due to using a fallback with a CPU trigger

3 participants