Implement CE Subscriptions filters

[devguyio]

Part of #5204
Rewrite of #5205 by @slinkydeveloper .

Proposed Changes

• 🎁 Add CE Subscriptions API filters field to Trigger objects.

Pre-review Checklist

• At least 80% unit test coverage
• E2E tests for any new behavior
• Docs PR for any user-facing impact
• Spec PR for any new API feature
• Conformance test for any change to the spec

Release Note

Triggers now include a CloudEvents Subscriptions API compatible filters field as an experimental feature.

Docs

Docs will be added with the implementation in mt-broker.

[devguyio]

This is ready for review, but I'm holding to figure out how should this be merged wrt the experimental feature in #5204 . I'm also still not clear on docs, spec and rest of the check boxes in the description.
/hold

[codecov]

Codecov Report

Merging #5715 (5363c2b) into main (31e4e2e) will increase coverage by 0.05%.
The diff coverage is 82.71%.

@@            Coverage Diff             @@
##             main    #5715      +/-   ##
==========================================
+ Coverage   81.99%   82.05%   +0.05%     
==========================================
  Files         220      220              
  Lines        7458     7527      +69     
==========================================
+ Hits         6115     6176      +61     
- Misses        916      917       +1     
- Partials      427      434       +7     

Impacted Files	Coverage Δ
pkg/apis/eventing/v1/trigger_types.go	100.00% <ø> (ø)
pkg/apis/eventing/v1/trigger_validation.go	85.61% <82.71%> (-6.17%)	⬇️
pkg/scheduler/statefulset/scheduler.go	78.59% <0.00%> (+0.63%)	⬆️
pkg/reconciler/subscription/subscription.go	80.23% <0.00%> (+1.01%)	⬆️
pkg/channel/message_dispatcher.go	81.25% <0.00%> (+1.56%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31e4e2e...5363c2b. Read the comment docs.

[antoineco]

If this is only adding API fields (and their validation) but the data plane implementation doesn't exist yet, is it really wise to add release notes already?

I'm also curious whether there is already a formal plan for implementing the "dialects" (part 2 of the feature).

[odacremolbap]

Nice to see this taking shape.
Did some early comments on it.

[devguyio]

If this is only adding API fields (and their validation) but the data plane implementation doesn't exist yet, is it really wise to add release notes already?

It was one of the questions I wanted to discuss, cause I also think it probably should only be added once the full experimental feature gets merged.

I'm also curious whether there is already a formal plan for implementing the "dialects" (part 2 of the feature).

Yes, in this section in the feature track which is also linked in the overarching issue #5204

[antoineco]

Overall this looks solid, I like what this API will enable 👍
I haven't looked at the tests yet, so just focusing on generic stuff and typos spotted in the doc so far.

[pierDipi]

If this is intended to be an experimental feature I think we should follow for consistency a similar pattern of #5149:

• The new field is not added to the OpenAPI schema
• The feature is disabled by default and uses a feature flag to enable it

[devguyio]

If this is intended to be an experimental feature I think we should follow for consistency a similar pattern of #5149:

• The new field is not added to the OpenAPI schema

@pierDipi I wasn't aware of the OpenAPI schema requirement, you are right! It was documented in our experimental feature doc 😅 .

• The feature is disabled by default and uses a feature flag to enable it

Yeah definitely, it's why I've a hold on this cause I'm still discovering the mechanics of how everything will land together but needed a feedback on the API shape. Cool!

[devguyio]

Updates:

• Applied review comments
• Added a feature flag

/unhold

[devguyio]

@pierDipi @antoineco changed. Thanks for the review.

[knative-metrics-robot]

The following is the coverage report on the affected files.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/eventing/v1/trigger_validation.go	98.3%	94.1%	-4.2

[pierDipi]

Let's merge this awesome feature!

/lgtm
/approve

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: antoineco, devguyio, pierDipi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [antoineco,devguyio,pierDipi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment