allow filtering on schema.GroupKind (#1080)

* allow filtering on schema.GroupKind

In addition deprecated usage of Filter with the introduction of
FilterGroupVersionKind

* reduce nesting & simplify boolean logic

controller/controller.go

@@ -86,15 +86,48 @@ func HandleAll(h func(interface{})) cache.ResourceEventHandler {
 // Filter makes it simple to create FilterFunc's for use with
 // cache.FilteringResourceEventHandler that filter based on the
 // schema.GroupVersionKind of the controlling resources.
+//
+// Deprecated: Use FilterGroupVersionKind or FilterGroupKind instead
 func Filter(gvk schema.GroupVersionKind) func(obj interface{}) bool {
+	return FilterGroupVersionKind(gvk)
+}
+
+// FilterGroupVersionKind makes it simple to create FilterFunc's for use with
+// cache.FilteringResourceEventHandler that filter based on the
+// schema.GroupVersionKind of the controlling resources.
+func FilterGroupVersionKind(gvk schema.GroupVersionKind) func(obj interface{}) bool {
 	return func(obj interface{}) bool {
-		if object, ok := obj.(metav1.Object); ok {
+		object, ok := obj.(metav1.Object)
-			owner := metav1.GetControllerOf(object)
+		if !ok {
-			return owner != nil &&
+			return false
-				owner.APIVersion == gvk.GroupVersion().String() &&
-				owner.Kind == gvk.Kind
 		}
-		return false
+
+		owner := metav1.GetControllerOf(object)
+		return owner != nil &&
+			owner.APIVersion == gvk.GroupVersion().String() &&
+			owner.Kind == gvk.Kind
+	}
+}
+
+// FilterGroupKind makes it simple to create FilterFunc's for use with
+// cache.FilteringResourceEventHandler that filter based on the
+// schema.GroupKind of the controlling resources.
+func FilterGroupKind(gk schema.GroupKind) func(obj interface{}) bool {
+	return func(obj interface{}) bool {
+		object, ok := obj.(metav1.Object)
+		if !ok {
+			return false
+		}
+
+		owner := metav1.GetControllerOf(object)
+		if owner == nil {
+			return false
+		}
+
+		ownerGV, err := schema.ParseGroupVersion(owner.APIVersion)
+		return err == nil &&
+			ownerGV.Group == gk.Group &&
+			owner.Kind == gk.Kind
 	}
 }
 

controller/controller_test.go

@@ -198,8 +198,8 @@ func TestFilterWithName(t *testing.T) {
 	}
 }
 
-func TestFilter(t *testing.T) {
+func TestFilterGroupKind(t *testing.T) {
-	filter := Filter(gvk)
+	filter := FilterGroupKind(gvk.GroupKind())
 
 	tests := []struct {
 		name  string
@@ -278,6 +278,126 @@ func TestFilter(t *testing.T) {
 			},
 		},
 		want: true,
+	}, {
+		name: "right owner reference, is controller, different version",
+		input: &Resource{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "foo",
+				Namespace: "bar",
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion: schema.GroupVersion{Group: gvk.Group, Version: "other"}.String(),
+					Kind:       gvk.Kind,
+					Controller: &boolTrue,
+				}},
+			},
+		},
+		want: true,
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := filter(test.input)
+			if test.want != got {
+				t.Errorf("Filter() = %v, wanted %v", got, test.want)
+			}
+		})
+	}
+}
+
+func TestFilterGroupVersionKind(t *testing.T) {
+	filter := FilterGroupVersionKind(gvk)
+
+	tests := []struct {
+		name  string
+		input interface{}
+		want  bool
+	}{{
+		name:  "not a metav1.Object",
+		input: "foo",
+		want:  false,
+	}, {
+		name:  "nil",
+		input: nil,
+		want:  false,
+	}, {
+		name: "no owner reference",
+		input: &Resource{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "foo",
+				Namespace: "bar",
+			},
+		},
+		want: false,
+	}, {
+		name: "wrong owner reference, not controller",
+		input: &Resource{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "foo",
+				Namespace: "bar",
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion: "another.knative.dev/v1beta3",
+					Kind:       "Parent",
+					Controller: &boolFalse,
+				}},
+			},
+		},
+		want: false,
+	}, {
+		name: "right owner reference, not controller",
+		input: &Resource{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "foo",
+				Namespace: "bar",
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion: gvk.GroupVersion().String(),
+					Kind:       gvk.Kind,
+					Controller: &boolFalse,
+				}},
+			},
+		},
+		want: false,
+	}, {
+		name: "wrong owner reference, but controller",
+		input: &Resource{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "foo",
+				Namespace: "bar",
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion: "another.knative.dev/v1beta3",
+					Kind:       "Parent",
+					Controller: &boolTrue,
+				}},
+			},
+		},
+		want: false,
+	}, {
+		name: "right owner reference, is controller",
+		input: &Resource{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "foo",
+				Namespace: "bar",
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion: gvk.GroupVersion().String(),
+					Kind:       gvk.Kind,
+					Controller: &boolTrue,
+				}},
+			},
+		},
+		want: true,
+	}, {
+		name: "right owner reference, is controller, wrong version",
+		input: &Resource{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "foo",
+				Namespace: "bar",
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion: schema.GroupVersion{Group: gvk.Group, Version: "other"}.String(),
+					Kind:       gvk.Kind,
+					Controller: &boolTrue,
+				}},
+			},
+		},
+		want: false,
 	}}
 
 	for _, test := range tests {