Skip to content
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

fix: Set StripPath to false in routes translated from HTTPRoute when ExpressionRoutes enabled #4359

Merged
merged 5 commits into from
Jul 18, 2023

Conversation

randmonkey
Copy link
Contributor

@randmonkey randmonkey commented Jul 17, 2023

What this PR does / why we need it:

Set StripPath to false to Kong routes translated from HTTPRoute when ExpressionRoutes enabled, to pass the gateway API conformance tests. The PR will also switch conformance tests to run with ExpressionRoutes enabled.

Which issue this PR fixes:

fixes #4352 and #4164
Special notes for your reviewer:

Also enables conformance to run for both traditional and expression routers. But the test titles

PR checks / conformance-tests / conformance-tests ([true|false]) 

looks weird. Any suggestions to display a clearer title on the page?

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@randmonkey randmonkey force-pushed the fix/expression_httproute_conformance_tests branch from 62336a4 to 1de664f Compare July 17, 2023 08:20
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Patch coverage: 100.0% and project coverage change: -0.1 ⚠️

Comparison is base (3cea973) 65.5% compared to head (61adcc4) 65.5%.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #4359     +/-   ##
=======================================
- Coverage   65.5%   65.5%   -0.1%     
=======================================
  Files        154     154             
  Lines      17735   17740      +5     
=======================================
- Hits       11630   11629      -1     
- Misses      5375    5379      +4     
- Partials     730     732      +2     
Impacted Files Coverage Δ
...rnal/dataplane/parser/translators/httproute_atc.go 96.0% <100.0%> (+<0.1%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@randmonkey randmonkey self-assigned this Jul 17, 2023
@randmonkey randmonkey added this to the KIC v2.11.0 milestone Jul 17, 2023
mlavacca
mlavacca previously approved these changes Jul 17, 2023
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jul 17, 2023
@randmonkey randmonkey marked this pull request as ready for review July 17, 2023 09:57
@randmonkey randmonkey added the ci/run-e2e Trigger e2e test run from PR label Jul 17, 2023
@team-k8s-bot
Copy link
Collaborator

E2E (targeted) tests with KIND-based clusters were started at https:/Kong/kubernetes-ingress-controller/actions/runs/5574265092

@team-k8s-bot team-k8s-bot removed the ci/run-e2e Trigger e2e test run from PR label Jul 17, 2023
Copy link
Member

@mlavacca mlavacca left a comment

Choose a reason for hiding this comment

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

Looks good, @randmonkey! Just a naming nitpick.

.github/workflows/_conformance_tests.yaml Show resolved Hide resolved
@mflendrich mflendrich mentioned this pull request Jul 17, 2023
32 tasks
@randmonkey randmonkey requested a review from mlavacca July 18, 2023 00:52
@randmonkey randmonkey merged commit 7d0b096 into main Jul 18, 2023
29 checks passed
@randmonkey randmonkey deleted the fix/expression_httproute_conformance_tests branch July 18, 2023 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run conformance tests with expression router enabled
4 participants