Add conformance test for combination of extended match types with core types #1984

gauravkghildiyal · 2023-05-01T00:59:49Z

/kind test
/area conformance

What type of PR is this?

What this PR does / why we need it:
Add conformance test for combination of extended match types with core types

Which issue(s) this PR fixes:

Fixes #1962

Does this PR introduce a user-facing change?:

NONE

k8s-ci-robot · 2023-05-01T00:59:58Z

Hi @gauravkghildiyal. 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 /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.

gauravkghildiyal · 2023-05-01T01:02:01Z

/area conformance

conformance/tests/httproute-matching-combinations.go

sunjayBhatia

test passes w/ Contour main 👍🏽

sunjayBhatia · 2023-05-01T15:20:54Z

/ok-to-test

shaneutt · 2023-05-01T22:53:46Z

/cc @shaneutt @mlavacca @arkodg

arkodg

LGTM thanks !

conformance/tests/httproute-matching-combinations.yaml

robscott

Thanks @gauravkghildiyal! I've got a couple non-blocking comments that don't need to block this PR, but could make for good follow ups here or in a future PR.

/approve

conformance/tests/httproute-matching-combinations.yaml

conformance/tests/httproute-matching-combinations.go

gauravkghildiyal

Thanks for the review Rob!

conformance/tests/httproute-matching-combinations.yaml

conformance/tests/httproute-matching-combinations.go

…e types

k8s-ci-robot · 2023-05-03T04:41:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arkodg, gauravkghildiyal, robscott, sunjayBhatia

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

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

robscott · 2023-05-17T23:59:20Z

Thanks @gauravkghildiyal!

/lgtm
/cherry-pick release-0.7

k8s-infra-cherrypick-robot · 2023-05-17T23:59:21Z

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

In response to this:

Thanks @gauravkghildiyal!

/lgtm
/cherry-pick release-0.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.

k8s-infra-cherrypick-robot · 2023-05-18T00:09:13Z

@robscott: new pull request created: #2032

In response to this:

Thanks @gauravkghildiyal!

/lgtm
/cherry-pick release-0.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.

sunjayBhatia · 2023-05-22T18:19:08Z

conformance/tests/httproute-query-param-matching.go

+				Namespace: ns,
+			},
+			{
+				Request:   http.Request{Headers: map[string]string{"version": "four"}, Path: "/?animal=hydra"},


Looks like this test case added after my review doesn't work with Contour

this assumes header matching gets precedence over method matching, but I'm not sure that is expressed anywhere explicitly in the spec, see #1993

Yes that was how I interpreted it -- given method matching is not in the list (and the only thing not in the list), it must have the least priority.

I'm sorry for causing any issues. I've sent #2052 as a temporary solution to help unblock (if this is the path we wish to take)

thanks! yeah not a super strong opinion on where it lands in terms of priority, but should probably be something explicitly debated/documented 👍🏽

k8s-ci-robot added kind/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 1, 2023

k8s-ci-robot requested review from howardjohn and sunjayBhatia May 1, 2023 00:59

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 1, 2023

k8s-ci-robot added the area/conformance-test Issues or PRs related to Conformance tests. label May 1, 2023

shaneutt added this to the v0.8.0 milestone May 1, 2023

sunjayBhatia reviewed May 1, 2023

View reviewed changes

conformance/tests/httproute-matching-combinations.go Outdated Show resolved Hide resolved

gauravkghildiyal force-pushed the httproute-match-combinations branch from 796f443 to e2dcaee Compare May 1, 2023 15:09

sunjayBhatia reviewed May 1, 2023

View reviewed changes

sunjayBhatia approved these changes May 1, 2023

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 1, 2023

gauravkghildiyal force-pushed the httproute-match-combinations branch from e2dcaee to 656efc4 Compare May 1, 2023 15:30

k8s-ci-robot requested review from arkodg, mlavacca and shaneutt May 1, 2023 22:53

arkodg approved these changes May 1, 2023

View reviewed changes

mlavacca reviewed May 2, 2023

View reviewed changes

conformance/tests/httproute-matching-combinations.yaml Outdated Show resolved Hide resolved

gauravkghildiyal force-pushed the httproute-match-combinations branch 2 times, most recently from c539210 to 62dd0f1 Compare May 2, 2023 17:59

robscott reviewed May 2, 2023

View reviewed changes

conformance/tests/httproute-matching-combinations.yaml Outdated Show resolved Hide resolved

conformance/tests/httproute-matching-combinations.go Outdated Show resolved Hide resolved

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 2, 2023

gauravkghildiyal force-pushed the httproute-match-combinations branch from 62dd0f1 to fda8a67 Compare May 3, 2023 04:20

gauravkghildiyal commented May 3, 2023

View reviewed changes

conformance/tests/httproute-matching-combinations.yaml Outdated Show resolved Hide resolved

conformance/tests/httproute-matching-combinations.go Outdated Show resolved Hide resolved

gauravkghildiyal force-pushed the httproute-match-combinations branch from fda8a67 to 1abb730 Compare May 3, 2023 04:26

gauravkghildiyal changed the title ~~Add conformance tests for checking combinations of multiple HTTPRoute…~~ Add conformance test for combination of extended match types with core types May 3, 2023

gauravkghildiyal added 2 commits May 2, 2023 21:40

Add conformance test for combination of extended match types with cor…

8ca6c59

…e types

Add execution bit to hack/verify-yamllint.sh

9d470b4

gauravkghildiyal force-pushed the httproute-match-combinations branch from 1abb730 to 9d470b4 Compare May 3, 2023 04:41

gauravkghildiyal requested a review from robscott May 17, 2023 01:48

k8s-ci-robot assigned robscott May 17, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 17, 2023

robscott modified the milestones: v0.8.0, v0.7.1 May 17, 2023

k8s-ci-robot merged commit 8913da1 into kubernetes-sigs:main May 18, 2023

k8s-infra-cherrypick-robot mentioned this pull request May 18, 2023

[release-0.7] Add conformance test for combination of extended match types with core types #2032

Merged

gauravkghildiyal deleted the httproute-match-combinations branch May 18, 2023 05:43

sunjayBhatia reviewed May 22, 2023

View reviewed changes

Add conformance test for combination of extended match types with core types #1984

Add conformance test for combination of extended match types with core types #1984

Uh oh!

Conversation

gauravkghildiyal commented May 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented May 1, 2023

Uh oh!

gauravkghildiyal commented May 1, 2023

Uh oh!

Uh oh!

sunjayBhatia left a comment

Choose a reason for hiding this comment

Uh oh!

sunjayBhatia commented May 1, 2023

Uh oh!

shaneutt commented May 1, 2023

Uh oh!

arkodg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robscott left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gauravkghildiyal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

k8s-ci-robot commented May 3, 2023

Uh oh!

robscott commented May 17, 2023

Uh oh!

k8s-infra-cherrypick-robot commented May 17, 2023

Uh oh!

k8s-infra-cherrypick-robot commented May 18, 2023

Uh oh!

sunjayBhatia May 22, 2023

Choose a reason for hiding this comment

Uh oh!

gauravkghildiyal May 22, 2023

Choose a reason for hiding this comment

Uh oh!

sunjayBhatia May 22, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

gauravkghildiyal commented May 1, 2023 •

edited

Loading