-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Emit Events for ClusterTriggerAuthentication #1647
Conversation
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
I don't really understand why we would do this? Those events aren't really useful for anything? It doesn't hurt per se but this implies you're trying to use events instead of an audit webhook for logging/recording/announcing cluster data changes. |
This can be useful outside of the scope of Kubernetes where you want to extend it or react in some other apps. That's more a general remark, not for these events (although I see its value), but why not be open; maybe somebody does cool stuff with them! |
It is more code to maintain and some small amount of increased load on the cluster if you are creating and deleting objects a lot. And the audit webhook system provides that kind of info for every object type, and a lot more info too (who did it, for example). |
Not strongly against these but it feels like it is redundant with stuff K8s already provides better via other means :) |
I can see your point, that these events aren't that imporant. But because we are already emitting the same events for the other objects, I think that we should have unified experience and this PR fixes that. The increased load and addition code is not that imho huge, we can think of this as a first spike to an additional related functionality. |
Agreed, we can still make it an opt-out if we have the need? |
Probably not even worth that, the maintenance is the bigger thing and even that is minimal. If we end up doing this for a bunch of types, we could make a shared generic version instead. That said, I'm not 100% sure this works? I don't think objects get the reconcile on delete if there isn't a finalizer to handle? |
Signed-off-by: Zbynek Roubalik zroubali@redhat.com
Kubernetes Events haven't been emitted for ClusterTriggerAuthentication, adding a controller the sam way it is done for TriggerAuthentication. Should be enough for now
Checklist
Fixes #1523