Skip to content

Attributes filters in Triggers#1643

Merged
knative-prow-robot merged 5 commits into
knative:masterfrom
nachocano:filter-attrib
Aug 5, 2019
Merged

Attributes filters in Triggers#1643
knative-prow-robot merged 5 commits into
knative:masterfrom
nachocano:filter-attrib

Conversation

@nachocano
Copy link
Copy Markdown
Contributor

@nachocano nachocano commented Aug 5, 2019 •
edited
Loading

Fixes #1610
This work is a subset of @grantr filtering PR. The credit should be his, although if I messed up porting the changes, then is my fault :)

Proposed Changes

  • Add attributes filter to the Trigger spec accepting a map of attribute keys and values to compare to the event context attributes, including extensions. Replaces sourceAndType.
  • As with sourceAndType, if a value in the attributes map is set to the empty string, then it will match anything.

Release Note

sourceAndType filtering on Triggers is deprecated. A new attributes filter is available, where besides filtering on standard CloudEvents attributes, it allows to filter based on extension attributes.

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Aug 5, 2019
@knative-prow-robot knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/test-and-release Test infrastructure, tests or release labels Aug 5, 2019
@nachocano
Copy link
Copy Markdown
Contributor Author

nachocano commented Aug 5, 2019

/assign @vaikas-google
/assign @Harwayne
/assign @grantr

It'd be great if we can get this one in before the cut, so that we finally remove the sourceAndType in 0.9. IMO it has been floating around for too long now.

@vaikas
Copy link
Copy Markdown
Contributor

vaikas commented Aug 5, 2019

Don't we need to pick the broker changes for the filter to take effect as well?
https://github.com/knative/eventing/pull/1152/files#diff-35094fef3ce16c43f7de43bf7d6c6556

Harwayne
Harwayne reviewed Aug 5, 2019
View reviewed changes
Comment thread pkg/apis/eventing/v1alpha1/trigger_types.go Outdated
Comment thread pkg/broker/receiver.go
Comment thread pkg/broker/receiver.go Outdated
Comment thread pkg/broker/receiver.go Outdated
Comment thread pkg/broker/receiver.go Outdated
Comment thread pkg/apis/eventing/v1alpha1/trigger_validation.go
@Harwayne
Copy link
Copy Markdown
Contributor

Harwayne commented Aug 5, 2019

Don't we need to pick the broker changes for the filter to take effect as well?
https://github.com/knative/eventing/pull/1152/files#diff-35094fef3ce16c43f7de43bf7d6c6556

Isn't that already here?

@vaikas
Copy link
Copy Markdown
Contributor

vaikas commented Aug 5, 2019

Don't we need to pick the broker changes for the filter to take effect as well?
https://github.com/knative/eventing/pull/1152/files#diff-35094fef3ce16c43f7de43bf7d6c6556

Isn't that already here?

whoa, I guess I totally just missed that, my apologies for the noise :(

Harwayne
Harwayne reviewed Aug 5, 2019
View reviewed changes
Comment thread pkg/apis/eventing/v1alpha1/trigger_validation.go Outdated
Comment thread pkg/apis/eventing/v1alpha1/trigger_validation_test.go
Harwayne
Harwayne reviewed Aug 5, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@Harwayne Harwayne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2019
@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented Aug 5, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Harwayne, nachocano

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 5, 2019
@knative-metrics-robot
Copy link
Copy Markdown

knative-metrics-robot commented Aug 5, 2019

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

File Old Coverage New Coverage Delta
pkg/apis/eventing/v1alpha1/trigger_validation.go 96.3% 97.6% 1.3
pkg/broker/receiver.go 86.1% 84.6% -1.5

@knative-prow-robot knative-prow-robot merged commit 58067b1 into knative:master Aug 5, 2019
@nachocano nachocano mentioned this pull request Aug 6, 2019
Closed
@grantr grantr mentioned this pull request Aug 29, 2019
Closed
pierDipi pushed a commit to pierDipi/eventing that referenced this pull request Mar 14, 2022
@openshift-cherrypick-robot @aliok
Different cron times for branches for periodics (knative#1639) (knati…
d086015 
…ve#1643)

* Different cron times for branches

* Wrap function invocations in quotes

* Add instructions

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

Co-authored-by: Ali Ok <aliok@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chaodaiG chaodaiG Awaiting requested review from chaodaiG

@evankanderson evankanderson Awaiting requested review from evankanderson

1 more reviewer

@Harwayne Harwayne Harwayne left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@grantr grantr

@Harwayne Harwayne

@vaikas vaikas

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release Test infrastructure, tests or release cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow Trigger Filter to filter on any Cloud Event Attribute (except data)

7 participants

@nachocano @vaikas @Harwayne @knative-prow-robot @knative-metrics-robot @grantr @googlebot