-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Dynamic setting value of TcpProxy.tunneling_config.hostname #19612
Comments
+1 for this. @lambdai mentioned that should be supported already but it seems it is not. Host header in this case I think should be mutable and we should relax this restriction. @lambdai @alyssawilk WDYT? |
Another solution could be to to use
WDYT? |
yeah 100% this would be useful |
Yes. That would also be a good option. |
@jewertow are you working on this? |
@ramaraochavali I didn't start yet, but I'm going to work on this the next week. But first I would like to dive into the implementation of TcpProxy and SNI inspector filter to figure out a better solution than just overwriting the host header. It's a hack rather than a good and long term solution, so I would like to suggest some more elegant API that will somehow inject SNI automatically when SNI inspector is enabled. What's more, it's also a potential feature for Istio egress gateway, so I have to discuss this idea with the Istio community. |
@jewertow Sure. thanks. This is also high priority for us - so wanted to know :-) |
There is an config verification that header name "host" or ":authority" can not be added or removed. I comment that line and it works. So I guess the only thing we need here is to keep maintain the restriction in http router but allow the mutation in tcp proxy tunnel setting |
I know that I suggested this workaround, but to be honest, I really don't like this idea.
This configuration is really bad, because:
Both cases are tricky and error prone. I don't think that it's a satisfying solution. I would suggest the following changes in this API:
Such an API would be backward compatible and cover most cases for tunneling TLS connections. Examples how would this API be used:
What do you think? |
@jewertow TBH, We do not need that much of complexity for our usecase. - we know SNI will be there and we setup that config that @lambdai shared for such cases only. But I do not have much knowledge on the different possible usecases with CONNECT - So may be @alyssawilk can weigh in here? |
Hello guys, As I understand it, the gateway should dynamically accept several (instead of a single static) hostnames and forward them accordingly via the TCP proxy. My question now is whether completely different target hosts (different IP addresses) can be addressed here, or whether it is a single host with several "vhosts". We would like to do following:
Sorry in advance if this is absolutely unrelated! |
I think we'd want to replace the old fixed hostname with a oneof, where one of the options could be downstream hostname (from SNI, or some pre-populated field), one of the options could be auto_sni (for CONNECT for DFP) and one of them could be static config. But I'd be interested in the thoughts of some of the @envoyproxy/api-shepherds , specifically @htuch and @mattklein123 |
Yeah I think deprecating and replacing with a oneof would be the right way of approaching this. It's not great but you could also relax the requirement that hostname be non-empty, and then add a oneof for the new values, and then just check in code that only 1 of them is set if you want to avoid a deprecation. |
+1 to #19612 (comment) |
Thank you for your feedback. I will submit a pull request with a prototype soon. |
@jewertow is adding AutoSni as a new choice other than setting constant hostname, see below, while still restricting Apart from this auto_sni, we should relax ":path"/":authority" for 2 reasons:
What do you think we relax the
|
we've been pretty strict about headers to add not including HTTP/2 "control" headers so folks vetting the control plane have clear knobs for setting host. I'd be mildly averse to doing that in one place and not others but cc @yanavlasov @htuch for thoughts |
I agree. The only reason I can think you might treat this differently is that in this case (I think) we're specifying the initial request as a client, rather than some filter randomly mutating in a HTTP filter pipeline. That said, I think it would be clearer to use explicit fields as needed for override and limit to only those that we have a use case for. |
@jewertow just a ping, you are working on this? We have been waiting for this. It seems there is a consensus on auto_sni? |
@ramaraochavali I'm sorry that it's not completed yet. I'm going to continue working on the implementation this week and I hope it will be ready to review by the end of this or next week. |
hi guys, i understand there will be an auto_sni option in tunneling_config - will it be bounded only to requestedServerName from the downstream connection info or we can use other things like downstreamDirectRemoteAddress ? |
@jewertow no worries. thank you |
Hi! Any updates on this feature? I'm also tracking this for an application. |
@pxpnetworks I submitted another pull request and explained why not |
@emanuelioan yes, there is a new PR. |
Somewhat high-level comment: per MASQUE proposal, I think it's necessary to support some URL template generation, but it really should be in |
I have similar needs but envoy is also doing TLS upstream to the final service. Which means the downstream request has no sni information. The downside is that a listener and a cluster must be replicated for each destination. |
…1067) Signed-off-by: Jacek Ewertowski [email protected] Commit Message: tcp_proxy: support command operators in tunneling_config.hostname Additional Description: This change enables dynamically setting tunneling_config.hostname with command operators. This pull request is an alternative for auto_sni. This change allows to configure TCP proxy as follows: ``` tunneling_config: hostname: %REQUESTED_SERVER_NAME%:443 ``` Risk Level: Low Testing: added unit tests Docs Changes: done Release Notes: done Platform Specific Features: none Fixes #19612 Fixes #21804
@jewertow many thanks for this contribution (and to anyone else who did contribute in one way or another). I was able to update my sample above to make it dynamic. |
@scrocquesel I'm glad I could help you. |
Just in case if anyone is looking for a working config ,I'm sharing which worked for me for
|
The documentation says that you can set hostname dynamically using e.g. dynamic metadata: But how can e.g. |
It assumes the usage of internal upstream transport socket to copy metadata from the endpoint metadata. |
@kyessenov, thank you. Is there anything else required beside this when routing to clusters:
- name: cluster_0
load_assignment:
cluster_name: cluster_0
endpoints:
- lb_endpoints:
- endpoint:
address:
envoy_internal_address:
server_listener_name: internal_listener
transport_socket:
name: envoy.transport_sockets.internal_upstream
typed_config:
"@type": type.googleapis.com/envoy.extensions.transport_sockets.internal_upstream.v3.InternalUpstreamTransport
passthrough_metadata:
- name: tunnel
kind: { host: {}}
transport_socket:
name: envoy.transport_sockets.raw_buffer
typed_config:
"@type": type.googleapis.com/envoy.extensions.transport_sockets.raw_buffer.v3.RawBuffer Because this setting doesn't seem to be working ( |
Yeah, you'd need actual metadata on the endpoint. https://github.com/istio/proxy/blob/master/testdata/cluster/internal_outbound.yaml.tmpl#L11. |
@kyessenov, do you mean something like this? clusters:
- name: cluster_0
load_assignment:
cluster_name: cluster_0
endpoints:
- lb_endpoints:
- endpoint:
address:
envoy_internal_address:
server_listener_name: internal_listener
metadata:
filter_metadata:
tunnel:
address: "my.host.com:80"
transport_socket:
name: envoy.transport_sockets.internal_upstream
typed_config:
"@type": type.googleapis.com/envoy.extensions.transport_sockets.internal_upstream.v3.InternalUpstreamTransport
passthrough_metadata:
- name: tunnel
kind: { host: {}}
transport_socket:
name: envoy.transport_sockets.raw_buffer
typed_config:
"@type": type.googleapis.com/envoy.extensions.transport_sockets.raw_buffer.v3.RawBuffer It doesn't seem to be working. Am I missing something? Also, I thought that this |
I think you got indentation wrong - metadata is on endpoints, not cluster. You can do a dynamic version. You'd need a couple of filters, one to capture address, and another to set the destination, communicating via filter state. You can see how we do it in Istio: |
You're right - but surprisingly, |
@kyessenov hi, can you help take a look #32103 |
Description:
I have a case where applications running in Kubernetes communicate with external services (outside Kubernetes and in another network) through a dedicated forward proxy. Apps send HTTP CONNECT request and the proxy opens a TCP tunnel. The flow is illustrated below.
I would like to make forward proxy transparent for apps by routing requests from applications via Envoy to forward proxy. This would allow to avoid sending HTTP CONNECT requests from apps. The desired solution is illustrated below.
It's quite easy to achieve the desired flow by creating the following listener:
The problem is that the
tunneling_config.hostname
can only be statically defined, so I can't tunnel traffic to multiple destinations having a single listener. One of the potential solutions would be to sethostname
based on SNI. TLS Inspector filter provides SNI value in%REQUESTED_SERVER_NAME%
, but it's not possible to overwriteHost
header.So what do you think about to make
Host
header mutable or to figure out another way to dynamically settunneling_config.hostname
?Related issues
#13809 #18838
The text was updated successfully, but these errors were encountered: