Skip to content

Comments

chore: bump k8s.io & controller-runtime dependencies#7455

Open
mycrEEpy wants to merge 16 commits intokedacore:mainfrom
mycrEEpy:update-controller-runtime
Open

chore: bump k8s.io & controller-runtime dependencies#7455
mycrEEpy wants to merge 16 commits intokedacore:mainfrom
mycrEEpy:update-controller-runtime

Conversation

@mycrEEpy
Copy link
Contributor

@mycrEEpy mycrEEpy commented Feb 15, 2026

… and:

  • implement generics in webhooks
  • implement new events recorder
  • add package action for common event actions
  • update k8s.io/* to v0.35.1
  • update sigs.k8s.io/controller-runtime to v0.23.1
  • update github.com/prometheus/common to v0.67.5 (required for expfmt.NewTextParser)

Checklist

  • When introducing a new scaler, I agree with the scaling governance policy
  • I have verified that my change is according to the deprecations & breaking changes policy
  • Tests have been added (if applicable)
  • Ensure make generate-scalers-schema has been run to update any outdated generated files
  • Changelog has been updated and is aligned with our changelog requirements, only when the change impacts end users
  • A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
  • A PR is opened to update the documentation on (repo) (if applicable)
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #7413

…nerics in webhooks

* update k8s.io/* to v0.35.1
* update sigs.k8s.io/controller-runtime to v0.23.1

Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
@mycrEEpy mycrEEpy requested a review from a team as a code owner February 15, 2026 11:31
@github-actions
Copy link

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 February 15, 2026 11:31
@snyk-io
Copy link

snyk-io bot commented Feb 15, 2026

⚠️ Snyk checks are incomplete.

Status Scanner Critical High Medium Low Total (0)
⚠️ Open Source Security 0 0 0 0 See details

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

@mycrEEpy mycrEEpy marked this pull request as draft February 15, 2026 12:18
required for expfmt.NewTextParser

Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
Copy link
Contributor Author

@mycrEEpy mycrEEpy left a comment

Choose a reason for hiding this comment

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

Should we ignore the deprecation of GetEventRecorderFor for now to not bloat this PR?


reader := strings.NewReader(bodyStr)
familiesParser := expfmt.TextParser{}
familiesParser := expfmt.NewTextParser(model.LegacyValidation)
Copy link
Contributor Author

@mycrEEpy mycrEEpy Feb 15, 2026

Choose a reason for hiding this comment

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

Also, unsure about which ValidationScheme to use here..

Copy link
Member

Choose a reason for hiding this comment

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

Which are the options?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

// LegacyValidation is a setting that requires that all metric and label names
// conform to the original Prometheus character requirements described by
// MetricNameRE and LabelNameRE.
LegacyValidation

// UTF8Validation only requires that metric and label names be valid UTF-8
// strings.
UTF8Validation

@mycrEEpy mycrEEpy marked this pull request as ready for review February 15, 2026 21:37
github.com/chzyer/logex => github.com/chzyer/logex v1.2.1

// pin k8s.io to v0.34.3 & sigs.k8s.io/controller-runtime to v0.22.4
// pin k8s.io to v0.35.1 & sigs.k8s.io/controller-runtime to v0.23.1
Copy link
Member

Choose a reason for hiding this comment

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

ensure that these deps have the same version outside the replace please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are all on v0.35.1 outside of the replace.
The controller-runtime already was on v0.23.1.
I have also fixed the client-go version which was set to v1.5.2, an old, deprecated version.

@JorTurFer
Copy link
Member

Should we ignore the deprecation of GetEventRecorderFor for now to not bloat this PR?

if it's possible, let's solve it here

Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
@keda-automation keda-automation requested a review from a team February 16, 2026 23:28
Comment on lines 94 to 109
@@ -107,7 +108,7 @@ func (h *scaleHandler) buildScalers(ctx context.Context, withTriggers *kedav1alp
return nil, err
}

Choose a reason for hiding this comment

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

Semgrep identified an issue in your code:
Variable scaler is likely modified and later used on error. In some cases this could result in panics due to a nil dereference

To resolve this comment:

🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.

💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by invalid-usage-of-modified-variable.

You can view more details about this finding in the Semgrep AppSec Platform.

Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
@mycrEEpy
Copy link
Contributor Author

Should we ignore the deprecation of GetEventRecorderFor for now to not bloat this PR?

if it's possible, let's solve it here

The new events.EventRecorder only exposes Eventf with two new parameters related runtime.Object and action string. I've set related to nil everywhere for now to keep it simple. For action I've added a new package with constants to represent common actions like create, update & delete.

Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
Signed-off-by: Tobias <bvrcreepyx@hotmail.de>
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.

Controller runtime breaking change v0.23+ for webhooks init

2 participants