NinesStack / sidecar

Gossip-based service discovery. Docker native, but supports non-container discovery, too.
MIT License
69 stars 7 forks source link

Skip Envoy HTTPConnectionManager cluster validation and simplify Envoy server code #56

Closed mihaitodor closed 4 years ago

mihaitodor commented 4 years ago

When I implemented the Envoy adapter, I didn't notice this validate_clusters field in the HTTP route configuration (see here). Although the docs say that "this setting default to false if the route table is loaded dynamically via the rds option", for some reason it seems to be set to true when the connection manager is of type HTTPConnectionManager and false when the connection manager is of type TcpProxy. The reason I added all that custom logic in server.go was to avoid Envoy spewing out errors and rejecting listeners for which clusters haven't been created yet because the go-control-plane sends listener and cluster updates to Envoy independently without a predefined order. With validate_clusters set to false, Envoy doesn't complain any more and it all seems to work fine with both Envoy v1.8.0 and v1.15.0.

Also, Istio seems to set this to false explicitly in their code when building RouteConfigurations: https://github.com/istio/istio/blob/f7fbc8043c3b3a1d020ac87d28334509003ef4c4/pilot/pkg/networking/core/v1alpha3/httproute.go#L61

Might be worth mentioning that the Envoy docs state that "if set to false and a route refers to a non-existent cluster, the route table will load and the router filter will return a 404 if the route is selected at runtime", which seems incorrect from my tests, because Envoy returns 503 instead of 404 in such cases.