-
Notifications
You must be signed in to change notification settings - Fork 74
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 v1 AdmissionReview #72
Comments
Indeed, Some months back I started adding support for it, but supporting both versions increased the complexity in the code and the API (a lot). On the other part, supporting Having said that, I'll not support both versions at the same time, I'll release a I've been thinking about this this past month and maybe I update Kubewebhook to Kubernetes |
That may have been the case back then, but I think it will make it harder for users (and I mean both end users and consumers of your library) to reliably move between versions (both webhook and Kubernetes), so I'd reconsider adding support for both versions. Otherwise, webhook developers and their users have to time their upgrades perfectly which might leave some of them stuck on older versions with potential bugs or unpatched security versions. Anyway, my two cents. Thanks for the library! |
Hi, @sagikazarmark! I started working on this, I'll try supporting both as you said, but for that, I need to refactor the library. This will be an |
Hi @slok, thanks for handling it. Is there any update when this be available? |
Hi @akpsgit! Sorry for the delay, I've been moving to a new house this and the past month, so... my plan is at Christmas, retake this and spent some part of the holidays on it, hoping in January will be ready. |
We''re also interested since returing warnings seems to be part of v1 |
@slok thanks for the update! |
Hi all! Just to let you know, this weekend I had some time and continue the work. I'm already handling webhooks in both versions with the same It will be a breaking change as I said (is Also, knowing that v2 will break the API (in favor of a better API and open to support N versions of webhooks), I've refactored the Prometheus metrics with better insights (e.g tell what is the version of the webhook, warnings...). On the release, I'll also provide Grafana dashboards for Kubewebhook V2 and some Prometheus alerts examples. I have some doubts about supporting tracing on Besides that, When v2 Is ready for a beta, I'll ask you if you can test it. so it helps to release a faster and more reliable v2 from day one. Many thanks! |
Hello there! I merged You can import with:
Apart from the examples and the godocs you can check k8s-webhook-example that has been already updated to use kubewebhook v2. I've didn't add support for tracing (Maybe I'll add it later) and Prometheus metrics aren't compatible anymore, better metrics but the old Grafana dashboard will break, I need some more time to get some dashboards and alerts. Thanks for all your help! cc/ @sagikazarmark @akpsgit @tpihl @sstoner @FrimIdan @florisvdg |
I just published a Grafana dashboard for |
Hello there! Anyone had the chance to check |
V2.0.0 released. Closing |
As pointed out in #55, kubewebhook does not support v1 AdmissionReview, but I assume at some point it will be inevitable to add support for it.
The text was updated successfully, but these errors were encountered: