envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.85k stars 4.78k forks source link

Envoy mobile old TCP listeners #31206

Closed Bfarkiani closed 8 months ago

Bfarkiani commented 10 months ago

Title: Compiling with Envoy mobile TCP listeners

Description: I saw in #24363 the original TCP listener was carved out and became the current API listener. Is it possible to compile envoy mobile with the old TCP listener? I saw there should be an "enable TCP listener" flag due to this comment

_Originally posted by @alyssawilk in https://github.com/envoyproxy/envoy/pull/24363#discussion_r1048799829_

But I couldn't find anything.

Thanks.

alyssawilk commented 10 months ago

you can compile it with the listener in, but there's no builder API to use it - you'd have to do custom yaml. Any reason you want E-M with a TCP listener?

Bfarkiani commented 10 months ago

@alyssawilk Thanks. I am actually using a custom yaml file for it. Could you please give me a hint as to which part of the build files I need to change? I intend to use it as a local proxy server that serves requests from other apps on the phone (a downstream listener-like envoy server).

alyssawilk commented 10 months ago

I think the config in proxy.kt here: https://github.com/envoyproxy/envoy/pull/31114/files#diff-fdad483120304741570df6891f9e9fd8ae200f88391eadddc822590c07851d37 is what you want. We removed e2e tests for this use case but not support. As long as you have the listener library linked into your build you should be fine.

Question - are you using any custom filters on your set up? We were planning on moving APIs to be proto only to reduce yaml use but we can leave it as a compile time opton if you're using that feature.

alyssawilk commented 10 months ago

sorry phrased that wrong - we'll definitely keep yaml support but ideally compile out by default, so I want to make sure it's not disruptive for you

Bfarkiani commented 10 months ago

I think the config in proxy.kt here: https://github.com/envoyproxy/envoy/pull/31114/files#diff-fdad483120304741570df6891f9e9fd8ae200f88391eadddc822590c07851d37 is what you want. We removed e2e tests for this use case but not support. As long as you have the listener library linked into your build you should be fine.

Question - are you using any custom filters on your set up? We were planning on moving APIs to be proto only to reduce yaml use but we can leave it as a compile time opton if you're using that feature.

Thank you @alyssawilk . I actually tested this configuration before. This one works with versions prior to December 5 but if you run it with current maven version implementation("io.envoyproxy.envoymobile:envoy:0.5.0.20231204") It crashes and returns an error: panic: Didn't find a registered implementation for name: 'envoy.listener_manager_impl.default Then we have to add this line:

listener_manager:
    name: envoy.listener_manager_impl.api
    typed_config:
      "@type": type.googleapis.com/envoy.config.listener.v3.ApiListenerManager

at the top of the configuration so it becomes:

listener_manager:
    name: envoy.listener_manager_impl.api
    typed_config:
      "@type": type.googleapis.com/envoy.config.listener.v3.ApiListenerManager           
static_resources:
  listeners:
  - name: base_api_listener
    address:
      socket_address: { protocol: TCP, address: 127.0.0.1, port_value: 10000 }
    api_listener:
      api_listener:
        "@type": "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.EnvoyMobileHttpConnectionManager"
        config:
          stat_prefix: api_hcm
          route_config:
            name: api_router
            virtual_hosts:
            - name: api
              domains: ["*"]
              routes:
              - match: { prefix: "/" }
                direct_response: { status: 400, body: { inline_string: "not found" } }
  - name: listener_proxy
    address:
      socket_address: { address: 127.0.0.1, port_value: 12000 }
    filter_chains:
      - 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: remote_hcm
            route_config:
              name: remote_route
              virtual_hosts:
              - name: remote_service
                domains: ["*"]
                routes:
                - match: { prefix: "/" }
                  route: { cluster: cluster_proxy }
              response_headers_to_add:
                - append_action: OVERWRITE_IF_EXISTS_OR_ADD
                  header:
                    key: x-proxy-response
                    value: 'true'
            http_filters:
              - name: envoy.filters.http.local_error
                typed_config:
                  "@type": type.googleapis.com/envoymobile.extensions.filters.http.local_error.LocalError
              - name: envoy.filters.http.dynamic_forward_proxy
                typed_config:
                  "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_forward_proxy.v3.FilterConfig
                  dns_cache_config: &dns_cache_config
                    name: base_dns_cache
                    dns_lookup_family: ALL
                    host_ttl: 86400s
                    dns_min_refresh_rate: 20s
                    dns_refresh_rate: 60s
                    dns_failure_refresh_rate:
                      base_interval: 2s
                      max_interval: 10s
                    dns_query_timeout: 25s
                    typed_dns_resolver_config:
                      name: envoy.network.dns_resolver.getaddrinfo
                      typed_config: {"@type":"type.googleapis.com/envoy.extensions.network.dns_resolver.getaddrinfo.v3.GetAddrInfoDnsResolverConfig"}
              - name: envoy.router
                typed_config:
                  "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
  clusters:
  - name: cluster_proxy
    connect_timeout: 30s
    lb_policy: CLUSTER_PROVIDED
    dns_lookup_family: ALL
    cluster_type:
      name: envoy.clusters.dynamic_forward_proxy
      typed_config:
        "@type": type.googleapis.com/envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig
        dns_cache_config: *dns_cache_config
layered_runtime:
  layers:
    - name: static_layer_0
      static_layer:
        envoy:
          # This disables envoy bug stats, which are filtered out of our stats inclusion list anyway
          # Global stats do not play well with engines with limited lifetimes
          disallow_global_stats: true

Which is useless since API listener does not listen to any port (and packets cannot be delivered to 127.0.0.1:12000) . That's why I actually asked to find a way to replace this API listener with TCP listener.

And for questions, yes we are using yaml configuration files. We actually are using envoy mobile similar to envoy server and we use our controller and xDS to update mobile configuration. That keeps overlay management much easier since we use similar configuration for all devices.

alyssawilk commented 10 months ago

Gotcha. Yeah the Maven build uses the default build which now compiles out listener code so it isn't supported in config. cc @abeyad for thoughts about adding it back in - we certainly don't want it for our use case but we don't use maven so it doesn't harm us to add it back, just bloats the binary a bit for folks who don't want it

abeyad commented 10 months ago

It sounds like you want to create a Maven artifact that includes the Envoy listener implementation (listener manager, etc) so you can listen for connections on sockets instead of just API calls? If so, I think that's fine, as long as it doesn't effect the standard Envoy Mobile Maven artifact. You'll want to conditionally include the listener manager in the code (so it doesn't effect the regular Envoy Mobile builds) and you'll want to update https://github.com/envoyproxy/envoy/blob/main/.github/workflows/mobile-release.yml to include a maven artifact w/ the listener code built in (similar to the maven artifact in mobile-release.yml that includes xds functionality). We don't use the Maven artifact, but I still think you'd want a separate artifact with the listener code built in, b/c most people wouldn't want that and it would just increase their binary size, which we're quite sensitive to.

Does that make sense?

Bfarkiani commented 10 months ago

@abeyad That makes sense. However my request was to bring old TCP listeners back in the main code, but I think because of the reasons you stated, this won't happen.
Thanks.

abeyad commented 10 months ago

@Bfarkiani we would rather not break your use case, so in the interim, we can just add the listener code back into the maven artifact that is published. After that, we'll work on publishing two different maven artifacts, one with the listener built in, and one without (so we don't suffer the binary size penalty).

Bfarkiani commented 10 months ago

@abeyad Great! Thank you.

github-actions[bot] commented 9 months ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

github-actions[bot] commented 8 months ago

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.