Skip to content
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

Support filters in EventPolicy resources #8114

Closed
Cali0707 opened this issue Jul 25, 2024 · 6 comments · Fixed by #8122
Closed

Support filters in EventPolicy resources #8114

Cali0707 opened this issue Jul 25, 2024 · 6 comments · Fixed by #8122
Assignees
Labels
kind/feature-request triage/accepted Issues which should be fixed (post-triage)

Comments

@Cali0707
Copy link
Member

Problem
To make the EventPolicy more powerful for users, we should add support for defining filters which run at ingress and determine which events pass. Given that the current behaviour for ingress of a component which has multiple EventPolicies applying to it is to have the event pass as long as it matches one of the policies, the way EventPolicies should be handled when there is a filter present is (in pseudocode):

for each policy:
    if sender in policy.from and policy.filters.filter(event):
        handle_event()
        break
reject_event()

Persona:
Which persona is this feature for? Developers, system administrators

Exit Criteria
E2E tests where events only pass ingress if they pass the filters

Time Estimate (optional):
How many developer-days do you think this may take to resolve? 3-5

Additional context (optional)
Add any other context about the feature request here.

@Cali0707
Copy link
Member Author

cc @pierDipi @Leo6Leo

@pierDipi
Copy link
Member

What happens when from is empty? Will the event be rejected ? and if so, would it work for the RequestReply use case #7912 ?

@Cali0707
Copy link
Member Author

I guess if from is empty but there are filters is that the event would be rejected (at least assuming no other policy allows the event to pass). To allow events from anywhere, you would need something like:

from:
  - sub: *
filters:
  cesql: ...

For the RequestReply use case, we would probably need two policies:

  1. With from as the wilcard match everything, and have a to go to the ingress path for the RequestReply resource.
  2. With from set to match the broker the RequestReply resource, and the filters set to have the new CESQL function we are adding. The to would then point to the reply path for the RequestReply resource.

Does this make sense @pierDipi ?

@pierDipi
Copy link
Member

For the RequestReply use case, we would probably need two policies:

  1. With from as the wilcard match everything, and have a to go to the ingress path for the RequestReply resource.
  2. With from set to match the broker the RequestReply resource, and the filters set to have the new CESQL function we are adding. The to would then point to the reply path for the RequestReply resource.

In YAML these 2 points translates into?

@Cali0707
Copy link
Member Author

I believe that the yamls would be something along the lines of:

apiVersion: eventing.knative.dev/v1alpha1
kind: EventPolicy
metadata:
  name: request-reply-ingress-policy
  namespace: knative-eventing
spec:
  to:
    - ref:
        apiVersion: "apps/v1"
        kind: Deployment
        name: request-reply
  from:
    - sub: *
  filters:
    cesql: "NOT KN_FROM_OIDC_IDENTITY(broker oidc identity)"
apiVersion: eventing.knative.dev/v1alpha1
kind: EventPolicy
metadata:
  name: request-reply-ingress-policy
  namespace: knative-eventing
spec:
  to:
    - ref:
        apiVersion: "apps/v1"
        kind: Deployment
        name: request-reply
  from:
    - sub: broker oidc identity
  filters:
    cesql: "KN_VERIFY_CORRELATION_ID(correlation_id, 'secret1', 'secret2')"
  # no filters, allow everything

Note: for this approach to work, we needed some way to say "not from a certain oidc identity". I did that by adding another CESQL function here. Another approach would be to allow specifyin relative URIs on the references in to, but that might become very complicated 🤷

@Cali0707
Copy link
Member Author

From offline discussions with @pierDipi , the EventPolicies would be:

  1. User supplied, no need for us to manage this one
  2. Referring to an API group internal.eventing.knative.dev/... to indicate that this is only for the internal reply. This policy would have the KN_VERIFY_CORRELATION_ID property set.

Using this approach, the proposed approach for event filters works, so I will be going forwards with that in a PR soon

/triage accepted
/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature-request triage/accepted Issues which should be fixed (post-triage)
Projects
No open projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants