-
Notifications
You must be signed in to change notification settings - Fork 468
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
conformance: add hostname intersection test #1144
conformance: add hostname intersection test #1144
Conversation
Hi @skriss. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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. |
/ok-to-test |
Adds a conformance test that validates hostname intersection betwen HTTPRoutes and Listeners. Updates kubernetes-sigs#1104. Signed-off-by: Steve Kriss <krisss@vmware.com>
1ae566f
to
2382f8a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
http.ExpectedResponse{ | ||
Request: http.ExpectedRequest{Host: "foo.bar.wildcard.io", Path: "/s2"}, | ||
StatusCode: 404, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x-ref #1159
http.ExpectedResponse{ | ||
Request: http.ExpectedRequest{Host: "too.many.anotherwildcard.io", Path: "/s4"}, | ||
StatusCode: 404, | ||
}, | ||
http.ExpectedResponse{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x-ref #1159
@robscott was that intended as a comment on one of my PRs or indeed this one with autocomplete being not helpful? Looks like the latter, doesn't look like this intersects with mine. |
Signed-off-by: Steve Kriss <krisss@vmware.com>
Updated based on #1159. |
@rainest sorry for the confusion, not sure what I did but likely an accidental @. |
1 similar comment
@rainest sorry for the confusion, not sure what I did but likely an accidental @. |
Thanks @skriss! This change LGTM and tests appear to be working as expected. Will leave a bit of time for someone else to chime in and add final LGTM. /approve |
1 similar comment
Thanks @skriss! This change LGTM and tests appear to be working as expected. Will leave a bit of time for someone else to chime in and add final LGTM. /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: robscott, skriss 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 |
1 similar comment
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: robscott, skriss 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 |
If possible, it'd be nice to get this merged before the v0.5.0 release. |
Thanks! /lgtm |
What type of PR is this?
/area conformance
What this PR does / why we need it:
Adds a conformance test that validates hostname
intersection between HTTPRoutes and Listeners.
Which issue(s) this PR fixes:
Updates #1104
Does this PR introduce a user-facing change?: