feat: allow disabling SNI matching on gateway#83
Merged
thesuperzapper merged 2 commits intoJan 27, 2024
Merged
Conversation
Signed-off-by: Mathew Wicks <[email protected]>
thesuperzapper
force-pushed
the
feat-disabling-SNI-matching-on-gateway
branch
from
January 27, 2024 22:54
a2af562 to
e017a45
Compare
Signed-off-by: Mathew Wicks <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR allows users to disable SNI matching on the gateway by adding a new value
deploykf_core.deploykf_istio_gateway.gateway.tls.matchSNI(which defaults totrue).Users may need to disable SNI matching when they are putting the deployKF gateway behind their own proxy which is doing TLS Termination, as most cloud proxies will not forward the original requests SNI to the backend system (e.g. AWS ALB). This is a very common issue, which even has its own Istio documentation page.
As part of this PR, we needed to work around the fact that Kubeflow Notebooks and Kubeflow TensorBoards currently always set their VirtualServices
spec.hoststo*. This is a problem because our dashboard's VirtualService matches all HTTP paths on the domain, and Envoy/Istio treats that route as "more specific" and so more preferred than the*notebook route, meaning that all notebook traffic incorrectly ends up at the dashboard pods.As a temporary measure until upstream kubeflow/kubeflow#6902 is merged and released, we resolve this issue by having a VirtualService that matches all
/notebook/paths which internally proxies the requests so that they have a different host/authority (notebooks-redirect.deploykf.example.com), which makes that request not match the dashboard route, and so end up at the notebook.Note that
notebooks-redirect.deploykf.example.comis NEVER served on the public-facing part of the gateway, and is only in-mesh (defined in the ServiceEntry calleddeploykf-istio-gateway), so there is no need to create a public DNS entry for it.NOTE: this must be merged AFTER: #82