-
Notifications
You must be signed in to change notification settings - Fork 612
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 Validating/Mutating webhook analyzer #548
feat: add Validating/Mutating webhook analyzer #548
Conversation
Signed-off-by: Rakshit Gondwal <rakshitgondwal3@gmail.com>
Signed-off-by: Rakshit Gondwal <rakshitgondwal3@gmail.com>
@rakshitgondwal would be nice to share what openai's response is as well based on the error messages you feed it |
|
I also left a few comments, thanks for your contribution ! |
Sorry, but I can't see your comments, can you please re check? |
Signed-off-by: Rakshit Gondwal <rakshitgondwal3@gmail.com>
Signed-off-by: Rakshit Gondwal <rakshitgondwal3@gmail.com>
Thank you for the merge! |
This reverts commit 750a10d.
Hi @rakshitgondwal this was my fault for merging entirely. Looking at the code can you explain what it's doing? Keep me honest here @arbreezy |
Oh yes @AlexsJones, even I am at fault here for misunderstanding the issue. I didn't think about this thoroughly once the PR was approved. Should have made a Draft PR first and then should've asked for feedback on the logic. But, after giving this a second thought, I had one question that should the analyzer be pointing to a service or a pod? Because in the WebhookConfiguration we mention the service that the webhook points to so how come would we be able to point to a pod? Correct me if I'm wrong here, and yes I'd like to fix this if you allow me. |
I was trying out this logic, but it is not working. I added a
|
I can take a look at implementing it a version of this and seeing whats wrong later today |
Closes #533
📑 Description
Changes made:
✅ Checks
Additional Information