-
Notifications
You must be signed in to change notification settings - Fork 6.4k
fix: make webhook payload handlers recover from panics #24862
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
fix: make webhook payload handlers recover from panics #24862
Conversation
Add a small guard that recovers and logs the panic and stack trace. Cover both the api-server and applicationset-controller webhooks. Signed-off-by: Jakub Ciolek <[email protected]>
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #24862 +/- ##
==========================================
+ Coverage 60.78% 60.83% +0.05%
==========================================
Files 404 351 -53
Lines 66232 60439 -5793
==========================================
- Hits 40256 36766 -3490
+ Misses 22730 20759 -1971
+ Partials 3246 2914 -332 ☔ View full report in Codecov by Sentry. |
@jake-ciolek are you seeing panics? if so, what is causing the webhook handlers to panic? |
@rumstead This is a follow up after those dropped last week: GHSA-gpx4-37g2-c8pv It prevents several classes of panics, shall they ever occur again (either through current or future weakness), from causing server restarts. |
got it, thanks! |
This makes the signature more explicit. Signed-off-by: Jakub Ciolek <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Signed-off-by: Jakub Ciolek <[email protected]>
Signed-off-by: Jakub Ciolek <[email protected]>
Signed-off-by: Jakub Ciolek <[email protected]>
🍒 Cherry-pick PR created for 3.2: #24912 |
🍒 Cherry-pick PR created for 3.0: #24913 |
🍒 Cherry-pick PR created for 3.1: #24914 |
❌ Cherry-pick failed for 2.14. Please check the workflow logs for details. |
@jake-ciolek do you have time to manually pick 2.14? |
Signed-off-by: Jakub Ciolek <[email protected]>
@crenshaw-dev Raised a pick for 2.14 here. Cheers. |
Add a small guard that recovers and logs the panic and stack trace.
Cover both the api-server and applicationset-controller webhooks.