Rework to apply CEL to filter the resources.

Signed-off-by: Kevin McDermott <[email protected]>

Author: bigkevmcd

api/v1/receiver_types.go

@@ -63,21 +63,14 @@ type ReceiverSpec struct {
 	// +optional
 	Events []string `json:"events,omitempty"`
 
-	// TODO: Validate one or other (or both?)
-
 	// A list of resources to be notified about changes.
 	// +required
 	Resources []CrossNamespaceObjectReference `json:"resources"`
 
-	// ResourceExpressions is a list of CEL expressions that will be parsed to
-	// determine resources to be notified about changes.
-	// The expressions must evaluate to CEL values that contain the keys "name",
-	// "kind", "apiVersion" and optionally "namespace".
-	//  These values will be parsed to CrossNamespaceObjectReferences.
-	// e.g. {"name": "test-resource-1", "kind": "Receiver", "apiVersion":
-	//       "notification.toolkit.fluxcd.io/v1"}.
-	// +optional
-	ResourceExpressions []string `json:"resourceExpressions,omitempty"`
+	// ResourceFilter is an expression that is applied to each Resource
+	// referenced in the Resources. If the expression returns false then the
+	// Resource is discarded and will not be notified.
+	ResourceFilter string `json:"resourceFilter,omitempty"`
 
 	// SecretRef specifies the Secret containing the token used
 	// to validate the payload authenticity.

api/v1/zz_generated.deepcopy.go

@@ -62,18 +62,12 @@ spec:
                   Secret references.
                 pattern: ^([0-9]+(\.[0-9]+)?(ms|s|m|h))+$
                 type: string
-              resourceExpressions:
+              resourceFilter:
                 description: |-
-                  ResourceExpressions is a list of CEL expressions that will be parsed to
-                  determine resources to be notified about changes.
-                  The expressions must evaluate to CEL values that contain the keys "name",
-                  "kind", "apiVersion" and optionally "namespace".
-                   These values will be parsed to CrossNamespaceObjectReferences.
-                  e.g. {"name": "test-resource-1", "kind": "Receiver", "apiVersion":
-                        "notification.toolkit.fluxcd.io/v1"}.
-                items:
-                  type: string
-                type: array
+                  ResourceFilter is an expression that is applied to each Resource
+                  referenced in the Resources. If the expression returns false then the
+                  Resource is discarded and will not be notified.
+                type: string
               resources:
                 description: A list of resources to be notified about changes.
                 items:

config/crd/bases/notification.toolkit.fluxcd.io_receivers.yaml

@@ -27,7 +27,6 @@ import (
 	"net/http/httptest"
 	"testing"
 
-	"github.com/go-logr/logr"
 	"github.com/google/go-github/v64/github"
 	"github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
@@ -762,7 +761,7 @@ func Test_handlePayload(t *testing.T) {
 			expectedResponseCode:       http.StatusOK,
 		},
 		{
-			name: "resources determined by CEL expressions",
+			name: "resources filtered with CEL expressions",
 			headers: map[string]string{
 				"Content-Type": "application/json; charset=utf-8",
 			},
@@ -775,11 +774,17 @@ func Test_handlePayload(t *testing.T) {
 					SecretRef: meta.LocalObjectReference{
 						Name: "token",
 					},
-					ResourceExpressions: []string{
-						`{"name": "test-resource-1", "kind": "Receiver", "apiVersion": "notification.toolkit.fluxcd.io/v1"}`,
-						`[{"name": body.image.split(':',2)[0] + '-2', "namespace": "tested", "kind": "Receiver", "apiVersion": "notification.toolkit.fluxcd.io/v1"}]`,
-						`body.resources.map(r, {"name": r, "kind": "Receiver", "apiVersion": "notification.toolkit.fluxcd.io/v1"})`,
+					Resources: []apiv1.CrossNamespaceObjectReference{
+						{
+							APIVersion: apiv1.GroupVersion.String(),
+							Kind:       apiv1.ReceiverKind,
+							Name:       "*",
+							MatchLabels: map[string]string{
+								"label": "production",
+							},
+						},
 					},
+					ResourceFilter: `resource.metadata.name in ["test-resource-1", "test-resource-2"]`,
 				},
 				Status: apiv1.ReceiverStatus{
 					WebhookPath: apiv1.ReceiverWebhookPath,
@@ -794,13 +799,6 @@ func Test_handlePayload(t *testing.T) {
 					"token": []byte("token"),
 				},
 			},
-			payload: map[string]interface{}{
-				"image": "test-resource:1.2.1",
-				"resources": []string{
-					"test-resource-3",
-					"test-resource-4",
-				},
-			},
 			resources: []client.Object{
 				&apiv1.Receiver{
 					TypeMeta: metav1.TypeMeta{
@@ -809,6 +807,9 @@ func Test_handlePayload(t *testing.T) {
 					},
 					ObjectMeta: metav1.ObjectMeta{
 						Name: "test-resource-1",
+						Labels: map[string]string{
+							"label": "production",
+						},
 					},
 				},
 				&apiv1.Receiver{
@@ -819,6 +820,9 @@ func Test_handlePayload(t *testing.T) {
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "test-resource-2",
 						Namespace: "tested",
+						Labels: map[string]string{
+							"label": "production",
+						},
 					},
 				},
 				&apiv1.Receiver{
@@ -828,6 +832,9 @@ func Test_handlePayload(t *testing.T) {
 					},
 					ObjectMeta: metav1.ObjectMeta{
 						Name: "test-resource-3",
+						Labels: map[string]string{
+							"label": "production",
+						},
 					},
 				},
 				&apiv1.Receiver{
@@ -837,10 +844,13 @@ func Test_handlePayload(t *testing.T) {
 					},
 					ObjectMeta: metav1.ObjectMeta{
 						Name: "test-resource-4",
+						Labels: map[string]string{
+							"label": "production",
+						},
 					},
 				},
 			},
-			expectedResourcesAnnotated: 4, // TODO: This should really check more than just the count.
+			expectedResourcesAnnotated: 2, // TODO: This should really check more than just the count.
 			expectedResponseCode:       http.StatusOK,
 		},
 		{
@@ -857,9 +867,17 @@ func Test_handlePayload(t *testing.T) {
 					SecretRef: meta.LocalObjectReference{
 						Name: "token",
 					},
-					ResourceExpressions: []string{
-						`{"name": ["test-resource-1"], "kind": "Receiver", "apiVersion": "notification.toolkit.fluxcd.io/v1"}`,
+					Resources: []apiv1.CrossNamespaceObjectReference{
+						{
+							APIVersion: apiv1.GroupVersion.String(),
+							Kind:       apiv1.ReceiverKind,
+							Name:       "*",
+							MatchLabels: map[string]string{
+								"label": "production",
+							},
+						},
 					},
+					ResourceFilter: `resource.name == "test-resource-1"`,
 				},
 				Status: apiv1.ReceiverStatus{
 					WebhookPath: apiv1.ReceiverWebhookPath,
@@ -874,13 +892,6 @@ func Test_handlePayload(t *testing.T) {
 					"token": []byte("token"),
 				},
 			},
-			payload: map[string]interface{}{
-				"image": "test-resource:1.2.1",
-				"resources": []string{
-					"test-resource-3",
-					"test-resource-4",
-				},
-			},
 			resources: []client.Object{
 				&apiv1.Receiver{
 					TypeMeta: metav1.TypeMeta{
@@ -889,11 +900,14 @@ func Test_handlePayload(t *testing.T) {
 					},
 					ObjectMeta: metav1.ObjectMeta{
 						Name: "test-resource-1",
+						Labels: map[string]string{
+							"label": "production",
+						},
 					},
 				},
 			},
 			expectedResourcesAnnotated: 0, // TODO: This should really check more than just the count.
-			expectedResponseCode:       http.StatusBadRequest,
+			expectedResponseCode:       http.StatusInternalServerError,
 		},
 	}
 
@@ -920,11 +934,11 @@ func Test_handlePayload(t *testing.T) {
 			}
 
 			client := builder.Build()
-			s := newReceiverHandler(
-				logger.NewLogger(logger.Options{}),
-				client,
-				false,
-			)
+			s := ReceiverServer{
+				port:       "",
+				logger:     logger.NewLogger(logger.Options{}),
+				kubeClient: client,
+			}
 
 			data, err := json.Marshal(tt.payload)
 			if err != nil {
@@ -962,71 +976,6 @@ func Test_handlePayload(t *testing.T) {
 	}
 }
 
-func TestReceiverServer(t *testing.T) {
-	receiver := &apiv1.Receiver{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "test-receiver",
-			Namespace: "default",
-		},
-		Spec: apiv1.ReceiverSpec{
-			Type: apiv1.GenericReceiver,
-			SecretRef: meta.LocalObjectReference{
-				Name: "token",
-			},
-			ResourceExpressions: []string{
-				`{"name": "test-receiver", "kind": "Receiver", "apiVersion": "notification.toolkit.fluxcd.io/v1"}`,
-			},
-		},
-		Status: apiv1.ReceiverStatus{
-			WebhookPath: apiv1.ReceiverWebhookPath,
-			Conditions: []metav1.Condition{
-				{
-					Type:   meta.ReadyCondition,
-					Status: metav1.ConditionTrue,
-				},
-			},
-		},
-	}
-	secret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "token",
-			Namespace: "default",
-		},
-		Data: map[string][]byte{
-			"token": []byte("token"),
-		},
-	}
-
-	k8sClient := buildTestClient(receiver, secret)
-
-	rs := newReceiverHandler(logr.Discard(), k8sClient, false)
-	srv := httptest.NewServer(rs)
-	defer srv.Close()
-
-	payload := map[string]any{
-		"image": "test-resource:1.2.1",
-	}
-
-	body, err := json.Marshal(payload)
-	if err != nil {
-		t.Fatal(err)
-	}
-	req, err := http.NewRequest(http.MethodPost, srv.URL+apiv1.ReceiverWebhookPath, bytes.NewBuffer(body))
-	if err != nil {
-		t.Fatal(err)
-	}
-	req.Header.Set("Content-Type", "application/json; charset=utf-8")
-
-	resp, err := srv.Client().Do(req)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if resp.StatusCode != http.StatusOK {
-		t.Errorf("got StatusCode %v, want %v", resp.StatusCode, http.StatusOK)
-	}
-}
-
 func buildTestClient(objs ...client.Object) client.Client {
 	scheme := runtime.NewScheme()
 	apiv1.AddToScheme(scheme)
@@ -1035,6 +984,5 @@ func buildTestClient(objs ...client.Object) client.Client {
 	return fake.NewClientBuilder().
 		WithScheme(scheme).
 		WithObjects(objs...).
-		WithStatusSubresource(&apiv1.Receiver{}).
 		WithIndex(&apiv1.Receiver{}, WebhookPathIndexKey, IndexReceiverWebhookPath).Build()
 }