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

listener/port sharing across protocols #527

Closed
kyessenov opened this issue Mar 2, 2017 · 16 comments
Closed

listener/port sharing across protocols #527

kyessenov opened this issue Mar 2, 2017 · 16 comments
Labels
enhancement Feature requests. Not bugs or questions.

Comments

@kyessenov
Copy link
Contributor

Using iptables rules and use_original_dst causes issues since multiple protocols get combined in a single listener section in Envoy configuration. For example, consider two micro-services "mysql" and "web" that listen on the same port 9000 but use TCP and HTTP. In a third service egress proxy config, we capture packets by redirecting to another listener with iptables and recover the original port with use_original_dst to handle both service traffic in listener on port 9000. We can distinguish between the two services by their virtual IPs: MYSQL_SERVICE_IP:9000 and WEB_SERVICE_IP:9000. However, tcp_proxy and http_connection_manager cannot coexist in together since tcp_proxy always terminates connections and does not fall through.

@mattklein123
Copy link
Member

mattklein123 commented Mar 3, 2017

Just jotting down notes from our Gitter discussion. There are a few ways we can go about this:

  1. We could make the tcp_proxy filter handle intermediate usage so that it falls through if there is no matching routing rule vs. closing the downstream connection.
  2. We could build a mechanism in which there is a "super" listener which makes routing decisions to a number of different filter chain stacks.
  3. We could build redirection capability into the network layer filter stack. Meaning, just like original_dst, but invoked via a filter via some type of routing rules, probably only during the new connection filter chain callback.

1 is the simplest, but probably less clean (though I could be convinced it is the right solution).
2 is very complicated, and I don't think worth the effort.
3 is more complicated, but would allow us to build some type of redirect filter to redirect a connection to a different listener. It's more complicated, but probably more clean and extensible.

@mattklein123 mattklein123 added the enhancement Feature requests. Not bugs or questions. label Mar 3, 2017
@rshriram
Copy link
Member

rshriram commented Mar 8, 2017

I would also like to suggest that we keep this feature as a low priority until we see real use cases where there are no workarounds or workarounds are clunky (i.e., services with different protocols and colliding ports (e.g., mysql:8080, web:8080). ). Given the relatively large number of ports available, it might be far easier to impose the restriction on the end user that one cannot have two services of different layer-7 protocols listening on the same port, be it http, mongo, redis, custom TCP, udp, etc. How common is it for people to run their redis servers on port 8080, backend oracle database on port 8080, and their NodeJS frontend on port 8080 as well ?

The state space of all possible configuration combinations in a generic distributed system framework is huge (be it kubernetes or mesos or some non container environment). Supporting all possible use cases (whether they are being really used by users) results in unnecessary complexity, instability and more potential for misconfiguration.

All said, the one neat feature that might be related to this, is the ability to do (a) http and https on same port (similar to standard web servers), (b) tcp/udp on same port (e.g., DNS over UDP and DNS over TCP), targeting the same upstream cluster in both cases. These two are far more common, and there are several examples around the web. However, supporting these use cases does not involve complicated work arounds. We could get away with a generic L4 listener type that does tcp/udp for same upstream cluster, and a generic http conn man that supports no tls and tls on same port.

@mattklein123
Copy link
Member

This is going to be covered by the v2 configuration/API for LDS. There will be "routing rules" that will allow selecting different filter stacks within a listener. I'm going to close this in favor of that work. cc @htuch

@zhaohuabing
Copy link
Member

@kyessenov @mattklein123 I run into the same problem when trying to integrate istio into our product. If I put the http connection manager before the tcp filter, will http connection manager fail through to the tcp filter, or it just terminate the connection?

@htuch
Copy link
Member

htuch commented Jan 6, 2019

@zhaohuabing HTTP connection manager will force L7 on the filter stack, there is no fallback behavior. Do you really need multiple protocols on the same port? If so, how would you like to see this fallback behavior work?

@zhaohuabing
Copy link
Member

zhaohuabing commented Jan 7, 2019

@htuch Istio complains port conflict when building outbound listeners if there are TCP and HTTP services on the same port. It simply abandons TCP services, which causes unexpected behaviours for the application.

Let's say we have a listener 0.0.0.0:9080, which only has an HTTP connection manager but will receive both HTTP and plain TCP traffic. Right now the plain TCP traffic is sent to the HTTP connection manager and then be terminated because it can't be handled as level 7.

I would like the HTTP connection manager fail through to the next network filter in the chain, then it will be handled by original dst cluster, routing the traffic to its original destination.


                           +---> HTTP connection manager +--->RDS(route:9080)
                           |
                           |            +
   listener:0.0.0.0:9080+--+            | fail through
                           |            |
                           |            v
                           +--->     TCP Proxy           +--->Original Cluster

@htuch
Copy link
Member

htuch commented Jan 7, 2019

@zhaohuabing how do you propose HTTP connection manager to detect that traffic is non-HTTP? At what point do you decide that this is a TCP connection vs. a badly formed HTTP request? Is this some heuristic on the first few bytes for example?

@zhaohuabing
Copy link
Member

zhaohuabing commented Jan 7, 2019

@htuch it's a good point. I think rather than fail through form HTTP connection manager, a more reasonable solution is to use a listener filter to tell the protocol before passing the request to a filter chain. Then we can split HTTP and TCP to different filter to handle them. Is this possible?

@htuch
Copy link
Member

htuch commented Jan 7, 2019

@zhaohuabing if we could allow listener filters to set arbitrary metadata and then add a metadata match criteria to the filter chain match, that would work as you describe. This seems like a good solution, I agree.

@zhaohuabing
Copy link
Member

We are trying to build a filter to identify the protocol and add other metadata such as HTTP host header, maybe we could override the protocol and servername metadata now using by TLS indication filter? However, it would be nice if this feature could be supported by envoy officially.

@ggreenway
Copy link
Contributor

When listener filters are run, the TLS handshake has not yet completed, so if this is a TLS connection, you cannot use any plaintext to set these values. If it's a plaintext connection, this would be feasible.

It seems like what you may need is a network-filter which determines the protocol, and then has multiple sub-filter-chains, one of which can be selected for any given connection.

@zhaohuabing
Copy link
Member

zhaohuabing commented Jan 7, 2019 via email

@htuch
Copy link
Member

htuch commented Jan 9, 2019

@ggreenway true. So, we want to set some metadata from arbitrary network filters and then match on this in HTTP connection manager... or we could have HCM still have a single filter chain and have a single match criteria which if not met make it pass-thru, allowing for multiple HCMs to be present at the end of a L4 filter chain.

Actually, thinking about this, we could have a special "metadata match L4 filter" which does two things:

  1. If the metadata matches, it applies a nested opaque L7 filter (which could be HCM).
  2. If there is no metadata match, it passes thru, allowing chaining.

This would then make this kind of matching possible for any network filter, e.g. Redis.

@freddygv
Copy link

freddygv commented Aug 7, 2019

@htuch was something like what @zhaohuabing discussed ever implemented? I'm seeing a few issues about this like but all of them closed without a resolution.

My goal is to expose an HTTP endpoint like /metrics and have all other TCP traffic go to the TCP proxy. (With the HTTP and TCP filters having different TLS contexts)

@mattklein123
Copy link
Member

lizan pushed a commit to lizan/envoy that referenced this issue Jun 4, 2020
* Add support for all headers statuses returned by WASM.

With this change, a WASM filter can return, say,
StopAllIterationAndWatermark. This is useful when a WASM filter
might want to call an external service and use the result to
change an HTTP header before continuing processing the body.

Signed-off-by: Gregory Brail <gregbrail@google.com>
@ciklista
Copy link

See https://www.envoyproxy.io/docs/envoy/latest/configuration/listener_filters/http_inspector. I think you could use this to do ^.

Thanks that was very helpful. Achieved something similar with the following config.

static_resources:
  listeners:
  - name: listener
    address:
      socket_address:
        address: 0.0.0.0
        port_value: 80
    # detect http protocoll
    listener_filters:
    - name: "envoy.filters.listener.http_inspector"
    filter_chains:
    # define two filter chains. One for the http route, another one for all other tcp traffic (rtsp in this case)
    - filter_chain_match: 
        # check if incomming protocoll is http/1.1
        # if so, continue with filter chain
        application_protocols: 
            - http/1.1
      filters:
      - name: envoy.filters.network.http_connection_manager
        typed_config:
          "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
          stat_prefix: ingress_http
          access_log:
          - name: envoy.access_loggers.stdout
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.access_loggers.stream.v3.StdoutAccessLog
          http_filters:
          - name: envoy.filters.http.router
          route_config:
            name: local_route
            virtual_hosts:
            - name: local_service
              domains: ["*"]
              routes:
              - match:
                  prefix: "/"
                route:
                  cluster: hls_cluster
    - filters:
        # else forward to other cluster
      - name: envoy.filters.network.tcp_proxy
        typed_config:
            "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
            stat_prefix: destination
            access_log:
              - name: envoy.access_loggers.stdout
                typed_config:
                    "@type": type.googleapis.com/envoy.extensions.access_loggers.stream.v3.StdoutAccessLog
            cluster: rtsp_cluster

  clusters:
  - name: hls_cluster
    type: LOGICAL_DNS
    dns_lookup_family: V4_ONLY
    load_assignment:
        cluster_name: hls_cluster
        endpoints:
        - lb_endpoints:
            - endpoint:
                address:
                    socket_address:
                        address: 127.0.0.1
                        port_value: 8888

  - name: rtsp_cluster
    connect_timeout: 30s
    type: LOGICAL_DNS
    dns_lookup_family: V4_ONLY
    load_assignment:
        cluster_name: rtsp_cluster
        endpoints:
        - lb_endpoints:
            - endpoint:
                address:
                    socket_address:
                        address: 127.0.0.1
                        port_value: 8554

jpsim pushed a commit that referenced this issue Nov 28, 2022
Description: #498 did not fully solve #492. The reset cleanly destructed all objects. However, because destruction was posted in to the event dispatcher, the event dispatcher was left with bad accesses.

This PR fixes the issue by issuing shutdown on the dispatcher, and only destructing once the event loop has exited and control has returned to the Engine's run function.

Risk Level: med - fixing crash on shutdown
Testing: local

Fixes #492

Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this issue Nov 29, 2022
Description: #498 did not fully solve #492. The reset cleanly destructed all objects. However, because destruction was posted in to the event dispatcher, the event dispatcher was left with bad accesses.

This PR fixes the issue by issuing shutdown on the dispatcher, and only destructing once the event loop has exited and control has returned to the Engine's run function.

Risk Level: med - fixing crash on shutdown
Testing: local

Fixes #492

Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions.
Projects
None yet
Development

No branches or pull requests

8 participants