Add SubscriptionsAPI filters to APIServerSource

[rh-hemartin]

This MR introduces the filters key in the APIServerSource Spec. This new field allows users to filter which messages are sent from the APIServerSource to the specified sink. The filter language is the new SubscriptionsAPI, that allows for powerful filtering.

• I changed a function to be exported in the broker/filter package. The other option I considered was to move the whole thing to a new package but it looked more disruptive.
• I followed the same approach that Triggers in https://github.com/knative/eventing/pull/5715 to introduce the field into the CRD (allow for everything).
• This is my first contribution here, so bear with me :rofl:

Fixes #7791

Proposed Changes

• :gift: Add new feature

Pre-review Checklist

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

Release Note

The filters field in APIServerSource is now beta and enabled by default

Docs

Docs #5916

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: rh-hemartin / name: Hector Martinez (52966bc9a3719c24cfb8629add0e35285c69d635)

[knative-prow[bot]]

Welcome @rh-hemartin! It looks like this is your first PR to knative/eventing 🎉

[knative-prow[bot]]

Hi @rh-hemartin. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[pierDipi]

Thanks @rh-hemartin, this is a really good PR

[rh-hemartin]

Can we create an e2e test similar to TestMTChannelBrokerNewTriggerFilters

https://github.com/knative/eventing/blob/2aa8985d582b20f07ef7f0ea3ef66ddee92585cb/test/rekt/new_trigger_filters_test.go#L34

I can't reply to your standalone comments, so I'm quoting you...

I'm having trouble with the eventshub image when it comes to the e2e tests. The eventshub image is not getting replaced in the installation process. To reproduce this I got:

1. Clone
1. `kind create cluster`
1. `export KO_DOCKER_REPO="kind.local"`
1. `./hack/install.sh`
1. `./hack/e2e-debug.sh TestApiServerSourceDataPlane_EventModes ./test/rekt`
1. Get errors because "message": "Back-off pulling image \"knative.dev/reconciler-test/cmd/eventshub\"""
1. `kind delete cluster`

How can I get the reconciler-test image uploaded and used? I tried with ko build vendor/knative.dev/reconciler-test/cmd/eventshub but it has some problems too:

Error: failed to publish images: importpath "ko://vendor/knative.dev/reconciler-test/cmd/eventshub/" is not supported: importpath is not `package main`

[Cali0707]

How can I get the reconciler-test image uploaded and used? I tried with ko build vendor/knative.dev/reconciler-test/cmd/eventshub but it has some problems too:

@rh-hemartin can you try running ./test/upload-test-images.sh ?

[rh-hemartin]

How can I get the reconciler-test image uploaded and used? I tried with ko build vendor/knative.dev/reconciler-test/cmd/eventshub but it has some problems too:

@rh-hemartin can you try running ./test/upload-test-images.sh ?

That is executed within ./hack/install.sh, so nothing different. Also the logs do not show any eventshub image being uploaded. It makes sense, because the eventshub is not on test/test_images, but on the vendor folder.

[pierDipi]

@rh-hemartin can you paste the full logs you're getting? On which OS are you on?

[pierDipi]

For this step:

./hack/e2e-debug.sh TestApiServerSourceDataPlane_EventModes ./test/rekt`

can you instead run:

SYSTEM_NAMESPACE=knative-eventing go test -tags=e2e -v -run TestApiServerSourceDataPlane_EventModes ./test/rekt

[rh-hemartin]

@rh-hemartin can you paste the full logs you're getting? On which OS are you on?

Hey! Getting all the logs together I think I found the problem, so thanks!

logger.go:146: 2024-03-28T14:40:14.795+0100	DEBUG	ko/cmd.go:55		github.com/google/[email protected] indirectly requires github.com/mitchellh/[email protected]: github.com/mitchellh/[email protected]: invalid version: git ls-remote -q origin in /home/hemartin/go/pkg/mod/cache/vcs/94ed57c5b21c953d93b47487113db43a5c9b69fd990329ec70dc77348c4dd443: exit status 128:	{"test": "TestApiServerSourceDataPlane_EventModes"}
    logger.go:146: 2024-03-28T14:40:14.795+0100	DEBUG	ko/cmd.go:55		remote: Repository not found.	{"test": "TestApiServerSourceDataPlane_EventModes"}
    logger.go:146: 2024-03-28T14:40:14.795+0100	DEBUG	ko/cmd.go:55		fatal: repository 'https://github.com/mitchellh/osext/' not found	{"test": "TestApiServerSourceDataPlane_EventModes"}
    logger.go:146: 2024-03-28T14:40:14.796+0100	WARN	environment/images.go:100	Ko publish failed, using image directly	{"test": "TestApiServerSourceDataPlane_EventModes", "error": "ko publish failed: exit status 1 -- command: [\"go\" \"run\" \"github.com/google/[email protected]\" \"publish\" \"-B\" \"knative.dev/reconciler-test/cmd/eventshub\"]", "image": "knative.dev/reconciler-test/cmd/eventshub"}

And I attached the full thing. I'm investigating this error and see if I can solve it...

full.log

Edit: Specifying GOOGLE_KO_VERSION=v0.15.2 for the test command solved the problem. Looks like the version specified in vendor/knative.dev/reconciler-test/pkg/images/ko/publish.go is broken (v0.11.2).

[rh-hemartin]

Hey, I will be away from my computer for three weeks, I will continue with this then. I'm now progressing on the e2e rekt test. I pushed the last changes, that included the run of ./hack/update-codegen.sh.

[pierDipi]

Edit: Specifying GOOGLE_KO_VERSION=v0.15.2 for the test command solved the problem. Looks like the version specified in vendor/knative.dev/reconciler-test/pkg/images/ko/publish.go is broken (v0.11.2).

I'm bumping the default version here https://github.com/knative-extensions/reconciler-test/pull/708

[rh-hemartin]

Can we create an e2e test similar to TestMTChannelBrokerNewTriggerFilters

https://github.com/knative/eventing/blob/2aa8985d582b20f07ef7f0ea3ef66ddee92585cb/test/rekt/new_trigger_filters_test.go#L34

I created a rekt test similar to that, I had to introduce changes to some places, as the apiserversource template and new methods to convert filters to strings, extracted from the trigger test implementation.

[pierDipi]

/ok-to-test

[Cali0707]

/test reconciler-tests

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 92.59259% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 69.26%. Comparing base (7e1c082) to head (52966bc). Report is 23 commits behind head on main.

Files	Patch %	Lines
pkg/broker/filter/filter_handler.go	50.00%	2 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7799      +/-   ##
==========================================
+ Coverage   69.22%   69.26%   +0.03%     
==========================================
  Files         339      341       +2     
  Lines       19494    15846    -3648     
==========================================
- Hits        13494    10975    -2519     
+ Misses       5337     4199    -1138     
- Partials      663      672       +9     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[pierDipi]

it looks like a flake

/test reconciler-tests

[pierDipi]

@rh-hemartin with the switch to disabled by default the tests are now failing, we need to add a new config-features configmap for tests with the feature enabled here https://github.com/knative/eventing/tree/main/test/config

[rh-hemartin]

@rh-hemartin with the switch to disabled by default the tests are now failing, we need to add a new config-features configmap for tests with the feature enabled here https://github.com/knative/eventing/tree/main/test/config

I did a copy of the features configmap from the production folder. That may impact more things than expected. That will need to be sync with the production configmap.

Would it be better to create the configmap but just include this feature flag? Then the default values will come from the code instead, maybe it will be better from the sync perspective. What do you think?

[rh-hemartin]

/test reconciler-tests

[pierDipi]

@rh-hemartin with the switch to disabled by default the tests are now failing, we need to add a new config-features configmap for tests with the feature enabled here https://github.com/knative/eventing/tree/main/test/config

I did a copy of the features configmap from the production folder. That may impact more things than expected. That will need to be sync with the production configmap.

Would it be better to create the configmap but just include this feature flag? Then the default values will come from the code instead, maybe it will be better from the sync perspective. What do you think?

Let's start with this, I think this flag disabled by default will be soon (3 months until the next release) become enabled by default as the underlying library is already part of a beta feature, so we will remove the CM soon as well

[knative-prow[bot]]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierDipi, rh-hemartin

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~~ [pierDipi]

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