emissary-ingress / emissary

open source Kubernetes-native API gateway for microservices built on the Envoy Proxy
https://www.getambassador.io
Apache License 2.0
4.39k stars 688 forks source link

feature: adopt best practice guidelines from Envoy on running as edge server #4663

Open LanceEa opened 2 years ago

LanceEa commented 2 years ago

Summary

This is based on a conversion in slack found here https://datawire-oss.slack.com/archives/CB46TNG83/p1666954516398899 from @psalaberria002.

The EnvoyProxy project provides best practice guidelines on running Envoy as an Edge Gateway. https://www.envoyproxy.io/docs/envoy/latest/configuration/best_practices/edge

For example:

{
 "http2_protocol_options":  {
    "max_concurrent_streams": 100,
    "initial_connection_window_size": 1048576,
    "initial_stream_window_size": 65536,
  }
}

We should do some research and design work for the following:

Note - We can use this issue for the initial design and discussions on the topic. If need be we can create separate issues for distinct independent tasks.

psalaberria002 commented 1 year ago

Hei! Sorry for the slow reply, busy times.

The recommended settings affect various parts of the system. The overload manager, listener and cluster buffer limits, connection limits, and some security related recommendations (normalizing paths, headers without underscores,etc).

As you mention, it makes sense to tackle some of these best practices separately.

I am particularly interested in the listener and cluster buffer limits. This is currently a blocker for some of our workloads that transfer large files over http2. The other features are more related to protecting Envoy from unknown clients, which should probably be addressed, but I would not prioritize those.

I do understand that Emissary wants to simplify how Envoy is configured, and it's there isn't a 1-1 mapping between Emissary and Envoy resources. However, for Listener and Cluster configurations we do have Listeners and Mappings. http2_protocol_options are present in both the Listener and the Clusters in Envoy, so I suggest we add something similar to the Emissary Listener. Every Mapping that has http2 enabled (grpc: True as of now), would inherit the http2 settings from the Listener. I don't think there is the need for adding a field for http2 options to the Mapping CRD, but that could also be useful.

When it comes to "ensuring we do not break existing behaviors", we have a couple of options. If we want to keep backwards compatibility without changing the default behavior, the new features would have to be opt-in, disabled by default, which I think it's ok for the http2 options. On the other hand, if we decide to enable the new features by default, we should make it easy for people to set them back to the old values.

Thoughts @LanceEa @AliceProxy ?

LanceEa commented 1 year ago

@psalaberria002 - Sorry for the slow response, fairly tied up the last few weeks and busy prepping for Kubecon EU so I'm not going to have much time to work on this but happy to provide guidance and feedback.

I'm happy for us to do this and as you outlined the key things for me are:

  1. We don't want to just blindly do a 1-1 to Envoy config because that would remove the benefit of Emissary reducing that complexity but sometimes it does makes sense
  2. We need to make sure we don't break a bunch of people unexpectedly

Can you outline the minimum settings that you would want to be able to set for both the downstream and upstream? If you already have a sample proposal feel free to outline that as well.

As far as I can tell, it sounds like you want to be able to set the http2_protocol_options in the HTTP Connection Manager which I would think more aligns with Emissary's Host since each Host will get its own FilterChain which would allow have different settings for each.

On the Cluster/Mapping, I need to think about that a little more and need to explore the grpc flag as well but I think potentially your solution might be a viable path forward.

@AliceProxy - any thoughts?