Fixed namespace checker to use the correct name #437
Conversation
Fixes #419 Apparently the wrong member was used to check the resource name. I don't know how resillient this patch is, but it works for most of the part.
| @@ -56,7 +56,8 @@ func (f NamespaceChecker) Run(object interface{}, event *events.Event) { | |||
| } | |||
| if botkubeConfig != nil { | |||
| for _, resource := range botkubeConfig.Resources { | |||
| if resource.Name == strings.ToLower(event.Kind) { | |||
| resourceName := strings.Split(strings.ToLower(event.Title), " ")[0] | |||
| if resource.Name == resourceName { | |||
There was a problem hiding this comment.
@hellozee we have a helper function implemented which returns GVR from given Kind https://github.com/infracloudio/botkube/blob/develop/pkg/utils/utils.go#L258. We can use that method to build "group/version/resource" or "version/resource" (if the group is nil) check
There was a problem hiding this comment.
This is not working for delete events since the resource type return by that method is /. Here is how I am trying to do it.
var eventObj *coreV1.Event
err := utils.TransformIntoTypedObject(object.(*unstructured.Unstructured), &eventObj)
if err != nil {
log.Errorf("Unable to transform object type: %v, into type: %v", reflect.TypeOf(object), reflect.TypeOf(eventObj))
return
}
tempResourceName, _ := utils.GetResourceFromKind(eventObj.InvolvedObject.GroupVersionKind())
resourceName := utils.GVRToString(tempResourceName)Also tbf this is not working for any events, though it might be my fault since the resource name is correct.
There was a problem hiding this comment.
@hellozee let's do this
- Add a new field
Resourcein the Event struct - Set the Resource value while creating New Event
- Refer event.Resource while building Title as well as in filters
Sounds good?
There was a problem hiding this comment.
Cool, will do that instead, 🐱
There was a problem hiding this comment.
@hellozee will you be able to implement the suggested changes?
There was a problem hiding this comment.
Ahh sorry, completely forgot about it, will do by this weekend, 😸
There was a problem hiding this comment.
Hey @hellozee, please let me know if you are working on this.
There was a problem hiding this comment.
whoops! expect a PR by evening for sure, 🙃
|
Done, but this doesn't work in case of a delete event. |
|
@hellozee do you mind adding the e2e test for the namespace checker filter (https://github.com/infracloudio/botkube/blob/develop/test/e2e/filters/filters.go#L44)? I am fine if we address that as a separate task. |
|
It would be better if someone else picks it. I cant promise anything this week, sorry. If no one picks it up this week, I would put up a patch next week, 🐱 |
|
Sounds good @hellozee |
|
Filed #443 to track. Merging this PR |
ISSUE TYPE
SUMMARY
Apparently the wrong member was used to check the resource name. I don't
know how resillient this patch is, but it works for most of the part.
Fixes #419