Eventing TLS: Add E2E TLS test for Parallel #7395
Conversation
knative-prow
bot
added
do-not-merge/work-in-progress
pierDipi
force-pushed
the
add-parallel-tls-test
branch
from
8370ac6 to
5fbf431
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7395 +/- ##
==========================================
+ Coverage 76.12% 76.16% +0.04%
==========================================
Files 260 260
Lines 14467 14456 -11
==========================================
- Hits 11013 11011 -2
+ Misses 2888 2879 -9
Partials 566 566 ☔ View full report in Codecov by Sentry. |
pierDipi
changed the title
knative-prow
bot
removed
the
do-not-merge/work-in-progress
|
/cherry-pick release-1.12 |
|
@pierDipi: once the present PR merges, I will cherry-pick it on top of release-1.12 in a new PR and assign it to you. In response to this:
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. |
|
/cherry-pick release-1.11 |
|
@pierDipi: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you. In response to this:
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. |
| @@ -102,26 +108,31 @@ func WithSubscriberAt(index int, ref *duckv1.KReference, uri string) manifest.Cf | |||
| } | |||
| subscriber := branch["subscriber"].(map[string]interface{}) | |||
|
|
|||
There was a problem hiding this comment.
Maybe check if d is nil here as well just like other functions like WithFilterAt do?
There was a problem hiding this comment.
d nil for subscriber, while possible, it doesn't make sense and this is test code, so it's ok to panic in that case
| Ref: p.Spec.Branches[branchNumber].Filter.Ref, | ||
| URI: p.Spec.Branches[branchNumber].Filter.URI, | ||
| } | ||
| r.Spec.Subscriber = p.Spec.Branches[branchNumber].Filter.DeepCopy() |
There was a problem hiding this comment.
Why we are making this change here?
There was a problem hiding this comment.
Destination has other fields beyond URI and Ref (which is why it wasn't working without this change) and we need to set them all, just better to assign the full copied struct instead of individual fields
|
/retest |
|
@pierDipi sorry for my late review. I just oversaw it. Can you rebase? |
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
pierDipi
force-pushed
the
add-parallel-tls-test
branch
from
5780b75 to
fcb71cf
Compare
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: creydr, 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:
Approvers can indicate their approval by writing |
|
@pierDipi: new pull request created: #7508 In response to this:
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: new pull request created: #7509 In response to this:
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. |
Add rekt TLS test for Parallel
/area test-and-release