-
Notifications
You must be signed in to change notification settings - Fork 137
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
Implement Receiver resource filtering with CEL #948
base: main
Are you sure you want to change the base?
Changes from all commits
3db236d
a7a2bcd
eba1e2e
2d84111
18b36a7
d38e2cc
c2b369d
4c50cd7
baada0a
a87623b
f986ccf
bae8794
e438ca1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -67,6 +67,11 @@ type ReceiverSpec struct { | |||||
// +required | ||||||
Resources []CrossNamespaceObjectReference `json:"resources"` | ||||||
|
||||||
// ResourceFilter is a CEL expression that is applied to each Resource | ||||||
// referenced in the Resources. If the expression returns false then the | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
// Resource will not be notified. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
ResourceFilter string `json:"resourceFilter,omitempty"` | ||||||
bigkevmcd marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
// SecretRef specifies the Secret containing the token used | ||||||
// to validate the payload authenticity. | ||||||
// +required | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -700,6 +700,117 @@ resources: | |||||||||
**Note:** Cross-namespace references [can be disabled for security | ||||||||||
reasons](#disabling-cross-namespace-selectors). | ||||||||||
|
||||||||||
#### Filtering reconciled objects with CEL | ||||||||||
|
||||||||||
To filter the resources that are reconciled you can use [Common Expression Language (CEL)](https://cel.dev/). | ||||||||||
|
||||||||||
For example to trigger `ImageRepositories` on notifications from [Google Artifact Registry](https://cloud.google.com/artifact-registry/docs/configure-notifications#examples) you can define a receiver. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
||||||||||
```yaml | ||||||||||
apiVersion: notification.toolkit.fluxcd.io/v1 | ||||||||||
kind: Receiver | ||||||||||
metadata: | ||||||||||
name: gar-receiver | ||||||||||
namespace: apps | ||||||||||
spec: | ||||||||||
type: gcr | ||||||||||
secretRef: | ||||||||||
name: flux-gar-token | ||||||||||
resources: | ||||||||||
- apiVersion: image.toolkit.fluxcd.io/v1beta2 | ||||||||||
kind: ImageRepository | ||||||||||
name: "*" | ||||||||||
matchLabels: | ||||||||||
registry: gar | ||||||||||
``` | ||||||||||
|
||||||||||
This will trigger the reconciliation of all `ImageRepositories` with matching labels `registry: gar`, but if you want to only notify `ImageRepository` resources that are referenced from the incoming hook you can use CEL to filter the resources. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
||||||||||
```yaml | ||||||||||
apiVersion: notification.toolkit.fluxcd.io/v1 | ||||||||||
kind: Receiver | ||||||||||
metadata: | ||||||||||
name: gar-receiver | ||||||||||
namespace: apps | ||||||||||
spec: | ||||||||||
type: gcr | ||||||||||
secretRef: | ||||||||||
name: flux-gar-token | ||||||||||
resources: | ||||||||||
- apiVersion: image.toolkit.fluxcd.io/v1beta2 | ||||||||||
kind: ImageRepository | ||||||||||
name: "*" | ||||||||||
matchLabels: | ||||||||||
registry: gar | ||||||||||
resourceFilter: 'request.body.tag.contains(resource.metadata.name)' | ||||||||||
``` | ||||||||||
|
||||||||||
If the body of the incoming hook looks like this: | ||||||||||
|
||||||||||
```json | ||||||||||
{ | ||||||||||
"action":"INSERT", | ||||||||||
"digest":"us-east1-docker.pkg.dev/my-project/my-repo/hello-world@sha256:6ec128e26cd5...", | ||||||||||
"tag":"us-east1-docker.pkg.dev/my-project/my-repo/hello-world:1.1" | ||||||||||
} | ||||||||||
``` | ||||||||||
|
||||||||||
This simple example would match `ImageRepositories` containing the name `hello-world`. | ||||||||||
|
||||||||||
If you want to do more complex processing: | ||||||||||
|
||||||||||
```yaml | ||||||||||
resourceFilter: has(resource.metadata.annotations) && request.body.tag.split('/').last().split(":").first() == resource.metadata.annotations['update-image'] | ||||||||||
``` | ||||||||||
|
||||||||||
This would look for an annotation "update-image" on the resource, and match it to the `hello-world` part of the tag name. | ||||||||||
|
||||||||||
**Note:** Currently the `resource` value in the CEL expression only provides the object metadata, this means you can access things like `resource.metadata.labels` and `resource.metadata.annotations` and `resource.metadata.name`. | ||||||||||
|
||||||||||
There are a number of functions available to the CEL expressions beyond the basic CEL functionality. | ||||||||||
|
||||||||||
The [Strings extension](https://github.com/google/cel-go/tree/master/ext#strings) is available. | ||||||||||
|
||||||||||
In addition the notifications-controller CEL implementation provides the following functions: | ||||||||||
|
||||||||||
#### first | ||||||||||
|
||||||||||
Returns the first element of a CEL array expression. | ||||||||||
|
||||||||||
``` | ||||||||||
<list<any>>.first() -> <any> | ||||||||||
``` | ||||||||||
|
||||||||||
This is syntactic sugar for `['hello', 'mellow'][0]` | ||||||||||
|
||||||||||
Examples: | ||||||||||
|
||||||||||
``` | ||||||||||
['hello', 'mellow'].first() // returns 'hello' | ||||||||||
[].first() // returns nil | ||||||||||
'this/test'.split('/').first() // returns 'this' | ||||||||||
``` | ||||||||||
|
||||||||||
#### last | ||||||||||
|
||||||||||
Returns the last element of a CEL array expression. | ||||||||||
|
||||||||||
``` | ||||||||||
<list<any>>.last() -> <any> | ||||||||||
``` | ||||||||||
|
||||||||||
Examples: | ||||||||||
|
||||||||||
``` | ||||||||||
['hello', 'mellow'].last() // returns 'mellow' | ||||||||||
[].last() // returns nil | ||||||||||
'this/test'.split('/').last() // returns 'test' | ||||||||||
``` | ||||||||||
|
||||||||||
This is syntactic sugar for `['hello', 'mellow'][size(['hello, 'mellow'])-1]` | ||||||||||
|
||||||||||
For zero-length array values, these will both return `nil`. | ||||||||||
|
||||||||||
### Secret reference | ||||||||||
|
||||||||||
`.spec.secretRef.name` is a required field to specify a name reference to a | ||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -156,6 +156,8 @@ func (r *ReceiverReconciler) Reconcile(ctx context.Context, req ctrl.Request) (r | |||||||||||||
// reconcile steps through the actual reconciliation tasks for the object, it returns early on the first step that | ||||||||||||||
// produces an error. | ||||||||||||||
func (r *ReceiverReconciler) reconcile(ctx context.Context, obj *apiv1.Receiver) (ctrl.Result, error) { | ||||||||||||||
log := ctrl.LoggerFrom(ctx) | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
// Mark the resource as under reconciliation. | ||||||||||||||
conditions.MarkReconciling(obj, meta.ProgressingReason, "Reconciliation in progress") | ||||||||||||||
|
||||||||||||||
|
@@ -166,6 +168,16 @@ func (r *ReceiverReconciler) reconcile(ctx context.Context, obj *apiv1.Receiver) | |||||||||||||
return ctrl.Result{Requeue: true}, err | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
if filter := obj.Spec.ResourceFilter; filter != "" { | ||||||||||||||
err := server.ValidateCELExpression(filter) | ||||||||||||||
if err != nil { | ||||||||||||||
conditions.MarkFalse(obj, meta.ReadyCondition, apiv1.InvalidCELExpressionReason, "%s", err) | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Marking the Ready condition to false results in the following status: status:
conditions:
- lastTransitionTime: "2024-11-04T16:07:03Z"
message: Reconciliation in progress
observedGeneration: 8
reason: ProgressingWithRetry
status: "True"
type: Reconciling
- lastTransitionTime: "2024-11-04T16:07:03Z"
message: |-
failed to parse expression request.body.tag.contains(: ERROR: <input>:1:27: Syntax error: mismatched input '<EOF>' expecting {'[', '{', '(', ')', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM_INT, NUM_UINT, STRING, BYTES, IDENTIFIER}
| request.body.tag.contains(
| ..........................^
observedGeneration: 8
reason: InvalidCELExpression
status: "False"
type: Ready
observedGeneration: 6 It says that the object is still being reconciled. This is due to how the reconciliation result is being interpreted in conditions.MarkStalled(obj, apiv1.InvalidCELExpressionReason, "%s", err) should delete any existing Reconciling condition, which will prevent the reconciling with retry condition reason to be set, refer notification-controller/internal/controller/receiver_controller.go Lines 215 to 218 in 2ebbf48
This should result in the status to look like: status:
conditions:
- lastTransitionTime: "2024-11-04T16:33:55Z"
message: |-
failed to parse expression request.body.tag.contains(: ERROR: <input>:1:27: Syntax error: mismatched input '<EOF>' expecting {'[', '{', '(', ')', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM_INT, NUM_UINT, STRING, BYTES, IDENTIFIER}
| request.body.tag.contains(
| ..........................^
observedGeneration: 8
reason: InvalidCELExpression
status: "True"
type: Stalled
- lastTransitionTime: "2024-11-04T16:07:03Z"
message: |-
failed to parse expression request.body.tag.contains(: ERROR: <input>:1:27: Syntax error: mismatched input '<EOF>' expecting {'[', '{', '(', ')', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM_INT, NUM_UINT, STRING, BYTES, IDENTIFIER}
| request.body.tag.contains(
| ..........................^
observedGeneration: 8
reason: InvalidCELExpression
status: "False"
type: Ready
observedGeneration: 6 Ready=False and Stalled=True looks good above. But notification-controller/internal/controller/receiver_controller.go Lines 220 to 221 in 2ebbf48
// Update observed generation when stalled.
if conditions.IsStalled(obj) {
patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
} This should result in the final status to be status:
conditions:
- lastTransitionTime: "2024-11-04T16:33:55Z"
message: |-
failed to parse expression request.body.tag.contains(: ERROR: <input>:1:27: Syntax error: mismatched input '<EOF>' expecting {'[', '{', '(', ')', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM_INT, NUM_UINT, STRING, BYTES, IDENTIFIER}
| request.body.tag.contains(
| ..........................^
observedGeneration: 8
reason: InvalidCELExpression
status: "True"
type: Stalled
- lastTransitionTime: "2024-11-04T16:07:03Z"
message: |-
failed to parse expression request.body.tag.contains(: ERROR: <input>:1:27: Syntax error: mismatched input '<EOF>' expecting {'[', '{', '(', ')', '.', '-', '!', 'true', 'false', 'null', NUM_FLOAT, NUM_INT, NUM_UINT, STRING, BYTES, IDENTIFIER}
| request.body.tag.contains(
| ..........................^
observedGeneration: 8
reason: InvalidCELExpression
status: "False"
type: Ready
observedGeneration: 8 Which I believe is correct and in alignment with what we do in other Flux APIs. Most of these conventions aren't documented anywhere, except for some github gists I created when we were trying to fit kstatus in flux API, for example https://gist.github.com/darkowlzz/969c90b2f309908a6d71dd861ba69653 which may be out of date by now. Please ask if anything looks incorrect above or any clarification is needed. |
||||||||||||||
obj.Status.WebhookPath = "" | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we perform this validation above first in this function, then we don't need to remove this status value here. In the case where the Receiver was good at first and got a bad configuration in a subsequent version, since the receiver handler checks the ready status of the receiver before processing requests, it should be okay to have a stale webhook path in the status. We do something similar in other flux APIs, keeping the previously successful result to just have a record, next to a failing status. But I don't see any issue in resetting it with empty value considering how the webhook path is used. |
||||||||||||||
log.Error(err, "parsing CEL resource filter expression") | ||||||||||||||
return ctrl.Result{}, nil | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
webhookPath := obj.GetWebhookPath(token) | ||||||||||||||
msg := fmt.Sprintf("Receiver initialized for path: %s", webhookPath) | ||||||||||||||
|
||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use fluxcd/pkg#859 when it's merged 👌