InMemoryChannel receiver validates received events #6511

pierDipi · 2022-09-06T16:20:13Z

IMC didn't run validation of the received events.

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

Proposed Changes

Validate events received by InMemoryChannels
Add conformance test

Pre-review Checklist

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

Release Note

InMemoryChannel receiver validates received events

Docs

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

codecov · 2022-09-06T16:28:10Z

Codecov Report

Merging #6511 (02e4ed5) into main (7f1f49f) will decrease coverage by 0.11%.
The diff coverage is 15.00%.

@@            Coverage Diff             @@
##             main    #6511      +/-   ##
==========================================
- Coverage   82.09%   81.98%   -0.12%     
==========================================
  Files         235      235              
  Lines       11714    11733      +19     
==========================================
+ Hits         9617     9619       +2     
- Misses       1628     1642      +14     
- Partials      469      472       +3

Impacted Files	Coverage Δ
pkg/channel/message_receiver.go	`64.22% <15.00%> (-9.82%)`	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

matzew · 2022-09-07T05:34:37Z

pkg/channel/message_receiver.go

+	bufferedMessage, err := buffering.CopyMessage(request.Context(), message)
+	if err != nil {
+		r.logger.Warn("Cannot buffer cloudevent message", zap.Error(err))
+		response.WriteHeader(nethttp.StatusBadRequest)
+		_ = r.reporter.ReportEventCount(&args, nethttp.StatusBadRequest)
+		return
+	}
+
+	event, err := binding.ToEvent(request.Context(), bufferedMessage)
+	if err != nil {
+		r.logger.Warn("failed to extract event from request", zap.Error(err))
+		response.WriteHeader(nethttp.StatusBadRequest)
+		_ = r.reporter.ReportEventCount(&args, nethttp.StatusBadRequest)
+		return
+	}
+
+	// run validation for the extracted event
+	if err := event.Validate(); err != nil {
+		r.logger.Warn("failed to validate extracted event", zap.Error(err))
+		response.WriteHeader(nethttp.StatusBadRequest)
+		return
+	}


could we move this section into a more encapsulated function?

what should that function return/do?

The binding, validation?

It's ok to inline and grow the functions, but I sometimes feel our funcs get longer and longer.

We can do in a separate PR, if we want to (of course not a blocker) 😅

matzew

/lgtm
/approve

knative-prow · 2022-09-07T08:38:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matzew, 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~~ [matzew,pierDipi]

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

matzew · 2022-09-07T08:48:12Z

/cherry-pick release-1.7

knative-prow-robot · 2022-09-07T08:48:13Z

@matzew: once the present PR merges, I will cherry-pick it on top of release-1.7 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.7

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.

matzew · 2022-09-07T08:48:18Z

/cherry-pick release-1.6

knative-prow-robot · 2022-09-07T08:48:19Z

@matzew: once the present PR merges, I will cherry-pick it on top of release-1.6 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.6

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.

matzew · 2022-09-07T08:48:26Z

/cherry-pick release-1.5

knative-prow-robot · 2022-09-07T08:48:28Z

@matzew: once the present PR merges, I will cherry-pick it on top of release-1.5 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.5

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.

matzew · 2022-09-07T08:48:34Z

/cherry-pick release-1.4

knative-prow-robot · 2022-09-07T08:48:35Z

@matzew: once the present PR merges, I will cherry-pick it on top of release-1.4 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.4

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.

knative-prow-robot · 2022-09-07T09:08:12Z

@matzew: new pull request created: #6512

In response to this:

/cherry-pick release-1.7

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.

knative-prow-robot · 2022-09-07T09:08:52Z

@matzew: new pull request created: #6513

In response to this:

/cherry-pick release-1.6

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.

knative-prow-robot · 2022-09-07T09:09:31Z

@matzew: new pull request created: #6514

In response to this:

/cherry-pick release-1.5

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.

knative-prow-robot · 2022-09-07T09:10:09Z

@matzew: new pull request created: #6515

In response to this:

/cherry-pick release-1.4

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 added 2 commits September 6, 2022 18:06

Validate events received by InMemoryChannels

d909274

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

Add conformance test

02e4ed5

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

matzew reviewed Sep 7, 2022

View reviewed changes

pierDipi changed the title ~~[WIP] InMemoryChannel receiver validates received events~~ InMemoryChannel receiver validates received events Sep 7, 2022

knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 7, 2022

matzew approved these changes Sep 7, 2022

View reviewed changes

knative-prow bot assigned matzew Sep 7, 2022

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Sep 7, 2022

knative-prow bot merged commit fae8544 into knative:main Sep 7, 2022

knative-prow-robot mentioned this pull request Sep 7, 2022

[release-1.7] InMemoryChannel receiver validates received events #6512

Merged

knative-prow-robot mentioned this pull request Sep 7, 2022

[release-1.6] InMemoryChannel receiver validates received events #6513

Merged

knative-prow-robot mentioned this pull request Sep 7, 2022

[release-1.5] InMemoryChannel receiver validates received events #6514

Merged

knative-prow-robot mentioned this pull request Sep 7, 2022

[release-1.4] InMemoryChannel receiver validates received events #6515

Merged

pierDipi deleted the imc-validate-events branch September 7, 2022 09:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

InMemoryChannel receiver validates received events #6511

InMemoryChannel receiver validates received events #6511

pierDipi commented Sep 6, 2022

codecov bot commented Sep 6, 2022 •

edited

Loading

matzew Sep 7, 2022

pierDipi Sep 7, 2022

matzew Sep 7, 2022

matzew Sep 7, 2022

matzew left a comment

knative-prow bot commented Sep 7, 2022

matzew commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

matzew commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

matzew commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

matzew commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

InMemoryChannel receiver validates received events #6511

InMemoryChannel receiver validates received events #6511

Conversation

pierDipi commented Sep 6, 2022

Proposed Changes

Pre-review Checklist

codecov bot commented Sep 6, 2022 • edited Loading

Codecov Report

matzew Sep 7, 2022

Choose a reason for hiding this comment

pierDipi Sep 7, 2022

Choose a reason for hiding this comment

matzew Sep 7, 2022

Choose a reason for hiding this comment

matzew Sep 7, 2022

Choose a reason for hiding this comment

matzew left a comment

Choose a reason for hiding this comment

knative-prow bot commented Sep 7, 2022

matzew commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

matzew commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

matzew commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

matzew commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

knative-prow-robot commented Sep 7, 2022

codecov bot commented Sep 6, 2022 •

edited

Loading