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: infraIR duplicate port translation for merged gateways #3061

Merged
merged 9 commits into from
Apr 7, 2024

Conversation

cnvergence
Copy link
Member

@cnvergence cnvergence commented Mar 29, 2024

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #2964

@cnvergence cnvergence force-pushed the fix-duplicate-port-validation branch from 1aa20e9 to 1caf9f3 Compare March 29, 2024 19:49
@cnvergence cnvergence changed the title fix: duplicate port translation for merged gateways fix: infraIR duplicate port translation for merged gateways Mar 29, 2024
Copy link

codecov bot commented Mar 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.48%. Comparing base (6941624) to head (7046f64).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3061      +/-   ##
==========================================
- Coverage   66.48%   66.48%   -0.01%     
==========================================
  Files         161      161              
  Lines       22647    22653       +6     
==========================================
+ Hits        15057    15060       +3     
- Misses       6717     6719       +2     
- Partials      873      874       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cnvergence cnvergence force-pushed the fix-duplicate-port-validation branch from 971b506 to 4e77360 Compare April 2, 2024 11:38
@cnvergence cnvergence marked this pull request as ready for review April 2, 2024 11:39
@cnvergence cnvergence requested a review from a team as a code owner April 2, 2024 11:39
Copy link
Member

@Xunzhuo Xunzhuo left a comment

Choose a reason for hiding this comment

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

Can add an individual testcase for this feature ?

@cnvergence
Copy link
Member Author

let's do that

Signed-off-by: Karol Szwaj <[email protected]>
@cnvergence cnvergence requested a review from Xunzhuo April 2, 2024 12:41
@cnvergence
Copy link
Member Author

/retest

@cnvergence
Copy link
Member Author

/retest

@cnvergence
Copy link
Member Author

/retest

@cnvergence cnvergence requested a review from arkodg April 4, 2024 10:16
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

@arkodg arkodg requested review from a team April 4, 2024 12:11
Copy link
Member

@Xunzhuo Xunzhuo left a comment

Choose a reason for hiding this comment

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

Thanks !

@Xunzhuo Xunzhuo merged commit 29946b0 into envoyproxy:main Apr 7, 2024
19 checks passed
arkodg pushed a commit to arkodg/gateway that referenced this pull request Apr 8, 2024
…xy#3061)

* fix: duplicate port translation for merged gateways

Signed-off-by: Karol Szwaj <[email protected]>

* refactor to map

Signed-off-by: Karol Szwaj <[email protected]>

* rename map

Signed-off-by: Karol Szwaj <[email protected]>

* add seperate testcase

Signed-off-by: Karol Szwaj <[email protected]>

---------

Signed-off-by: Karol Szwaj <[email protected]>
(cherry picked from commit 29946b0)
Signed-off-by: Arko Dasgupta <[email protected]>
Xunzhuo added a commit that referenced this pull request Apr 8, 2024
* Run certgen when upgrading (#2934)

run certgen when upgrading

Signed-off-by: huabing zhao <[email protected]>
(cherry picked from commit 62ecf15)
Signed-off-by: Arko Dasgupta <[email protected]>

* Fix: nil secret in resourceversiontable (#2982)

* fix nil secret in resourceversiontable

Signed-off-by: huabing zhao <[email protected]>

* check secrets in the xds result

Signed-off-by: huabing zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
(cherry picked from commit e880439)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: add missing http filters to the http filter chain (#2970)

* fix: add missing http filters to the http filter chain

Signed-off-by: huabing zhao <[email protected]>

* refactor

Signed-off-by: huabing zhao <[email protected]>

* fix lint

Signed-off-by: huabing zhao <[email protected]>

* add comments

Signed-off-by: huabing zhao <[email protected]>

* remove refactor

Signed-off-by: huabing zhao <[email protected]>

* remove refactor

Signed-off-by: huabing zhao <[email protected]>

* fix gen

Signed-off-by: huabing zhao <[email protected]>

* fix lint

Signed-off-by: Huabing Zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
(cherry picked from commit f699edf)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: allow websockets in url rewrite (#3022)

allow websockets in url rewrite

Signed-off-by: Jesse Haka <[email protected]>
Co-authored-by: zirain <[email protected]>
(cherry picked from commit 3d51933)
Signed-off-by: Arko Dasgupta <[email protected]>

* Set host for http health checker explicitly to avoid using the cluster name as host header for http health checking request. (#3057)

* Set host for http health checker explictly to avoid using the cluster name as host header for http health checking request

Signed-off-by: lemonlinger <[email protected]>

* fix broken tests

Signed-off-by: lemonlinger <[email protected]>

* fix health-check test case in xds translation

Signed-off-by: lemonlinger <[email protected]>

* Simplify code and concise comments

Signed-off-by: lemonlinger <[email protected]>

---------

Signed-off-by: lemonlinger <[email protected]>
(cherry picked from commit 8f450a9)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: do not create infra resources when missing translated listeners (#3043)

* fix: do not create infra resources when missing translated listeners

Signed-off-by: Karol Szwaj <[email protected]>

* remove empty line

Signed-off-by: Karol Szwaj <[email protected]>

* skip infra creation on empty listeners and log it

Signed-off-by: Karol Szwaj <[email protected]>

---------

Signed-off-by: Karol Szwaj <[email protected]>
(cherry picked from commit 36d7141)
Signed-off-by: Arko Dasgupta <[email protected]>

* Fix: double slashes in redirect URL (#2998)

* fix: double trailing splashs in redirect URL

Signed-off-by: huabing zhao <[email protected]>

* add e2e tests

Signed-off-by: huabing zhao <[email protected]>

* fix lint

Signed-off-by: huabing zhao <[email protected]>

* fix test

Signed-off-by: huabing zhao <[email protected]>

* fix test

Signed-off-by: huabing zhao <[email protected]>

* fix test

Signed-off-by: huabing zhao <[email protected]>

* fix test

Signed-off-by: huabing zhao <[email protected]>

* add e2e tests

Signed-off-by: huabing zhao <[email protected]>

* fix test

Signed-off-by: huabing zhao <[email protected]>

* revert

Signed-off-by: huabing zhao <[email protected]>

* use regex rewrite to generate the redirect url

Signed-off-by: huabing zhao <[email protected]>

* use regex rewrite to generate the redirect url

Signed-off-by: huabing zhao <[email protected]>

* use regex rewrite to generate the redirect url

Signed-off-by: huabing zhao <[email protected]>

* remove comments

Signed-off-by: huabing zhao <[email protected]>

* extract method

Signed-off-by: huabing zhao <[email protected]>

* address comments

Signed-off-by: huabing zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
(cherry picked from commit ceb697f)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: Allow Policy to attach to multiple http listeners  (#2967)

* Fixing the clienttrafficpolicy validation.

Signed-off-by: Lior Okman <[email protected]>

* Make SecurityPolicy validate correctly.

Signed-off-by: Lior Okman <[email protected]>

* Reverted the SecurityPolicy validation - handled differently via
another feature.

Signed-off-by: Lior Okman <[email protected]>

* Updated the tests to reflect that this validation isn't required for SecurityPolicy

Signed-off-by: Lior Okman <[email protected]>

* Added some comments to explain the validation being performed.

Signed-off-by: Lior Okman <[email protected]>

* Updated the error message as requested in the review.

Signed-off-by: Lior Okman <[email protected]>

---------

Signed-off-by: Lior Okman <[email protected]>
(cherry picked from commit f9409e4)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: set path prefix for http ext auth service (#3018)

Signed-off-by: huabing zhao <[email protected]>
(cherry picked from commit 2882b7c)
Signed-off-by: Arko Dasgupta <[email protected]>

* Change route sorting order to Exact > RegularExpression > PathPrefix (#2579)

* Change route sorting order to Exact > RegularExpression > PathPrefix

kubernetes-sigs/gateway-api#1770
kubernetes-sigs/gateway-api#1855

Signed-off-by: Stéphane Cottin <[email protected]>
(cherry picked from commit 11f56fd)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: infraIR duplicate port translation for merged gateways (#3061)

* fix: duplicate port translation for merged gateways

Signed-off-by: Karol Szwaj <[email protected]>

* refactor to map

Signed-off-by: Karol Szwaj <[email protected]>

* rename map

Signed-off-by: Karol Szwaj <[email protected]>

* add seperate testcase

Signed-off-by: Karol Szwaj <[email protected]>

---------

Signed-off-by: Karol Szwaj <[email protected]>
(cherry picked from commit 29946b0)
Signed-off-by: Arko Dasgupta <[email protected]>

* translator: set SpawnUpstreamSpan to true (#3102)

* translator: set SpawnUpstreamSpan to true

Signed-off-by: zirain <[email protected]>

* update

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 635ebfc)
Signed-off-by: Arko Dasgupta <[email protected]>

* fix: rate limit doesn't work with two(and more) listeners (#3085)

* fix: rate limit doesn't work with two listeners

Signed-off-by: huabing zhao <[email protected]>

* add e2e test for rate limit on multiple listeners

Signed-off-by: huabing zhao <[email protected]>

* address comments

Signed-off-by: huabing zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
Co-authored-by: Xunzhuo <[email protected]>
(cherry picked from commit a5bedbc)
Signed-off-by: Arko Dasgupta <[email protected]>

* rerun make testdata

Signed-off-by: Arko Dasgupta <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Jesse Haka <[email protected]>
Signed-off-by: lemonlinger <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Lior Okman <[email protected]>
Signed-off-by: Stéphane Cottin <[email protected]>
Signed-off-by: zirain <[email protected]>
Co-authored-by: Huabing Zhao <[email protected]>
Co-authored-by: Jesse Haka <[email protected]>
Co-authored-by: zirain <[email protected]>
Co-authored-by: Meng <[email protected]>
Co-authored-by: Karol Szwaj <[email protected]>
Co-authored-by: Lior Okman <[email protected]>
Co-authored-by: vixns <[email protected]>
Co-authored-by: Xunzhuo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: duplicate port definition when using mergeGateway
3 participants