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(MeshGateway): ensure that duplicate listeners are not added when crossMesh is enabled on a listener and Routes specify hostnames #8156

Merged

Conversation

ttreptow
Copy link
Contributor

@ttreptow ttreptow commented Oct 26, 2023

In the cross-mesh case, the SNI string will be a kuma SNI string for the gateway service (e.g. edge-gateway{mesh=default,port=tcp-8080}). Thus it is not possible to distinguish hosts at the listener level and no filter chain sni matchers are added. This can lead to a duplicate listener filter chain being added if there are multiple hostnames to route.

Thus we truncate the gatewayHosts array to size 1 before creating the listener blocks.

Fixes #8076

Supersedes #8105

Checklist prior to review

  • Link to relevant issue as well as docs and UI issues --
  • This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as a image registry) and it will work on Windows, system specific functions like syscall.Mkfifo have equivalent implementation on the other OS --
  • Tests (Unit test, E2E tests, manual test on universal and k8s) --
    • Don't forget ci/ labels to run additional/fewer tests
  • Do you need to update UPGRADE.md? --
  • Does it need to be backported according to the backporting policy? (this GH action will add "backport" label based on these file globs, if you want to prevent it from adding the "backport" label use no-backport-autolabel label) -- I don't think this needs to be backported

@ttreptow ttreptow requested a review from a team as a code owner October 26, 2023 12:49
@ttreptow ttreptow requested review from jakubdyszkiewicz and lukidzi and removed request for a team October 26, 2023 12:49
@ttreptow ttreptow force-pushed the fix/remove-extra-cross-mesh-chains branch from 3898e77 to 9f58886 Compare October 26, 2023 12:52
@ttreptow
Copy link
Contributor Author

I accidentally re-incorporated @michaelbeaumont 's changes while rebasing 🙈 . I'm going to push a fixed commit in a minute

@ttreptow ttreptow force-pushed the fix/remove-extra-cross-mesh-chains branch 3 times, most recently from f09a3e7 to e03462e Compare October 26, 2023 13:09
@lahabana lahabana requested review from michaelbeaumont and removed request for jakubdyszkiewicz October 26, 2023 13:23
@michaelbeaumont michaelbeaumont changed the title Ensure that duplicate listeners are not added when crossMesh is enabled on a listener and Routes specify hostnames fix: ensure that duplicate listeners are not added when crossMesh is enabled on a listener and Routes specify hostnames Oct 26, 2023
@ttreptow ttreptow changed the title fix: ensure that duplicate listeners are not added when crossMesh is enabled on a listener and Routes specify hostnames fix(MeshGateway): ensure that duplicate listeners are not added when crossMesh is enabled on a listener and Routes specify hostnames Oct 26, 2023
@michaelbeaumont
Copy link
Contributor

@ttreptow unfortunately you need to rebase and add the --signoff to satisfy the DCO

…ed on a listener and Routes specify hostnames

In the cross-mesh case, the SNI string will be a kuma SNI string for the gateway service (e.g. edge-gateway{mesh=default,port=tcp-8080}). Thus it is not possible to distinguish hosts at the listener level and no filter chain sni matchers are added. This can lead to a duplicate listener filter chain being added if there are multiple hostnames to route.

Thus we truncate the gatewayHosts array to size 1 before creating the listener blocks.

Signed-off-by: Tim Treptow <ttreptow@domaintools.com>
@ttreptow ttreptow force-pushed the fix/remove-extra-cross-mesh-chains branch from dc1f331 to 6dd05f1 Compare October 31, 2023 21:49
Copy link
Contributor

@michaelbeaumont michaelbeaumont left a comment

Choose a reason for hiding this comment

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

Thanks @ttreptow !

@michaelbeaumont michaelbeaumont added the ci/run-full-matrix PR: Runs all possible e2e test combination (expensive use carefully) label Nov 1, 2023
@michaelbeaumont michaelbeaumont merged commit 8cc520e into kumahq:master Nov 1, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/run-full-matrix PR: Runs all possible e2e test combination (expensive use carefully)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CrossMesh MeshGateway listener does not work when MeshGatewayRoute hostnames are set
2 participants