-
Notifications
You must be signed in to change notification settings - Fork 1.6k
✨ Add support for custom webhook paths #5171
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
✨ Add support for custom webhook paths #5171
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
509ce87 to
6f034cb
Compare
|
Hey @damsien Could you please give a hand in this one? |
cc396a5 to
7aeaee9
Compare
|
c/c @sbueringer |
7aeaee9 to
074203f
Compare
Users can now set custom paths for webhooks using --defaulting-path and --validation-path flags. Requires controller-runtime v0.20+. Assisted-by: Cursor
074203f to
767d0db
Compare
|
/test pull-kubebuilder-e2e-k8s-1-34-0 |
|
Hello @camilamacedo86 , thanks for reaching me! I tested your implementation and it works well! However, I noticed that you didn't implemented an regex checker for the path. I don't know if it is supposed to not being implemented. If we need to add the regex validation, I would like to contribute to your PR :) |
|
Hi @daniem,
Thank you for raising and validating this — great work! 🥇 It seems that from the Kubernetes side, any path is accepted, so I’m not entirely sure what kind of validations we could enforce. Our convention: That said, since this is already the default behavior, suggesting it might be redundant — so I’m not completely sure it’s worth enforcing. Given that you’ve verified everything is implemented accurately, I’m happy for us to move forward — great job! 🚀 |
|
To be more precise, I was talking about a regex that checks if the path is a valid path (no weird characters, etc...). The regex validation is already done in controller-runtime: https://github.com/kubernetes-sigs/controller-runtime/blob/67242822fa14d36e8ef7a5f7933022334b11a976/pkg/builder/webhook.go#L374-L383. Since controller-runtime already implement it, if the user put a wrong path in the kubebuilder command, then he gonna have an error on runtime. So it is not strictly necessary to add the regex validation on the kubebuilder side but still it can be useful :) |
|
/test pull-kubebuilder-e2e-k8s-1-32-0 |
1 similar comment
|
/test pull-kubebuilder-e2e-k8s-1-32-0 |
Users can now set custom paths for webhooks using --defaulting-path and --validation-path flags. Requires controller-runtime v0.21+.
Closes: #4295