Skip to content
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.

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

feat: add support to validating webhooks #352

Merged
merged 4 commits into from
Jan 13, 2023

Conversation

JorTurFer
Copy link
Member

@JorTurFer JorTurFer commented Jan 2, 2023

Signed-off-by: Jorge Turrado [email protected]

To pass CI checks here, the feature needs to be merged in KEDA because the new image is required for CI checks here

Checklist

  • I have verified that my change is according to the deprecations & breaking changes policy
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • README is updated with new configuration values (if applicable)
  • A PR is opened to update KEDA core (repo) (if applicable, ie. when deployment manifests are modified)

Fixes kedacore/keda#3755
Fixes kedacore/keda#3087
Fixes kedacore/keda#3083
Fixes kedacore/keda#3088

@JorTurFer JorTurFer requested a review from a team as a code owner January 2, 2023 00:29
@tomkerkhove tomkerkhove marked this pull request as draft January 3, 2023 07:13
@JorTurFer JorTurFer force-pushed the validation-hook branch 4 times, most recently from 32fb043 to 955b37d Compare January 10, 2023 21:54
@JorTurFer JorTurFer changed the title WIP - feat: add support to validating webhooks feat: add support to validating webhooks Jan 10, 2023
@JorTurFer JorTurFer marked this pull request as ready for review January 10, 2023 23:26
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

looking good, just nits

keda/templates/14-keda-deployment.yaml Show resolved Hide resolved
keda/templates/33-webhooks-servicemonitor.yaml Outdated Show resolved Hide resolved
Signed-off-by: Jorge Turrado <[email protected]>
@JorTurFer JorTurFer merged commit 32caf60 into kedacore:main Jan 13, 2023
@JorTurFer JorTurFer deleted the validation-hook branch January 13, 2023 12:15
@JorTurFer
Copy link
Member Author

f**k! I thought I was clicking in auto-merge but I this repo doesn't have it because there isn't required checks, I hope all will pass, if not I'll solve the issues in other PR

@@ -52,6 +56,23 @@ metricsServer:
# - keda-operator-metrics-apiserver
# topologyKey: "kubernetes.io/hostname"

webhooks:
enabled: true # This value will be removed in keda v2.12
Copy link
Member

Choose a reason for hiding this comment

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

Why will this be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because we talked about making this not optional in KEDA, Am I wrong?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have to solve the issues I generated with the early merge, I can remove the comment if you want

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment